Merge branch 'master' into dynamic-parent-cgroups

8 files changed, 246 insertions, 217 deletions

diff --git a/.github/workflows/lint-test-build-push.yaml b/.github/workflows/lint-test-build-push.yaml
index d77ead1..338e301 100644
--- a/.github/workflows/lint-test-build-push.yaml
+++ b/.github/workflows/lint-test-build-push.yaml
@@ -81,37 +81,27 @@ jobs:
           tags: ghcr.io/python-discord/snekbox-venv:${{ steps.sha_tag.outputs.tag }}
 
       - name: Start Container
-        run: >-
-          docker run
-          --tty
-          --detach
-          --name snekbox_test
-          --privileged
-          --hostname pdsnk-dev
-          -e PYTHONDONTWRITEBYTECODE=1
-          -e PIPENV_PIPFILE='/snekbox/Pipfile'
-          --volume "${PWD}":"${PWD}"
-          --workdir "${PWD}"
-          --entrypoint /bin/bash
-          ghcr.io/python-discord/snekbox-venv:${{ steps.sha_tag.outputs.tag }}
+        run: |
+          export IMAGE_SUFFIX='-venv:${{ steps.sha_tag.outputs.tag }}'
+          docker-compose up --no-build -d
 
       # One of the unit tests needs to import numpy.
       - name: Install dependencies
         run: >-
-          docker exec snekbox_test /bin/bash -c
+          docker exec snekbox_dev /bin/bash -c
           'pipenv install --system --deploy --dev && pip install numpy'
 
       # Required by pre-commit.
       - name: Install git
         run: >-
-          docker exec snekbox_test /bin/bash -c
+          docker exec snekbox_dev /bin/bash -c
           'apt-get -y update && apt-get install -y git=1:2.20.*'
 
       # pre-commit's venv doesn't work with user installs.
       # Skip the flake8 hook because the following step will run it.
       - name: Run pre-commit hooks
         run: >-
-          docker exec snekbox_test /bin/bash -c
+          docker exec snekbox_dev /bin/bash -c
           'PIP_USER=0 SKIP=flake8 pre-commit run --all-files'
 
       # This runs `flake8` in the container and asks `flake8` to output
@@ -121,7 +111,7 @@ jobs:
       # wrong where.
       - name: Run linter
         run: >-
-          docker exec snekbox_test /bin/bash -c
+          docker exec snekbox_dev /bin/bash -c
           'flake8 --format
           "::error file=%(path)s,line=%(row)d,col=%(col)d::[flake8] %(code)s: %(text)s"'
 
@@ -135,7 +125,7 @@ jobs:
         run: |
           echo '::set-output name=started::true'
           cmd='coverage run -m unittest; coverage report -m'
-          docker exec snekbox_test /bin/bash -c "${cmd}"
+          docker exec snekbox_dev /bin/bash -c "${cmd}"
 
       # Set-up a Python version to process the coverage reports
       # Note: This step runs even if the test step failed to make
diff --git a/DEVELOPING.md b/DEVELOPING.md
new file mode 100644
index 0000000..8b0799f
--- /dev/null
+++ b/DEVELOPING.md
@@ -0,0 +1,104 @@
+# Development Environment
+
+## Initial Setup
+
+A Python 3.9 interpreter and the [pipenv] package are required. Once those requirements are satisfied, install the project's dependencies:
+
+```
+pipenv sync --dev
+```
+
+Follow that up with setting up the pre-commit hook:
+
+```
+pipenv run precommit
+```
+
+Now Flake8 will run and lint staged changes whenever an attempt to commit the changes is made. Flake8 can still be invoked manually:
+
+```
+pipenv run lint
+```
+
+## Running snekbox
+
+Use Docker Compose to start snekbox in development mode. The optional `--build` argument can be passed to force the image to be rebuilt.
+
+```
+docker-compose up
+```
+
+The container has all development dependencies. The repository on the host is mounted within the container; changes made to local files will also affect the container.
+
+Note that the compose file depends on the environment variable `PWD` being set to the current working directory. It needs it to create the aforementioned bind mount. Unix shells normally have this set already. If for some reason it is not set, it needs to be manually set. A convenient way to set it is to define it in a `.env` file which Docker Compose will automatically read.
+
+To build a normal container that can be used in production, run
+
+```
+pipenv run build
+```
+
+Refer to the [README] for how to run the container normally.
+
+## Running Tests
+
+Tests are run through coverage.py using unittest. To run the tests within a development container, run
+
+```
+pipenv run test
+```
+
+## Coverage
+
+To see a coverage report, run
+
+```
+pipenv run report
+```
+
+Alternatively, a report can be generated as HTML with
+
+```
+pipenv run coverage html
+```
+
+The HTML will output to `./htmlcov/` by default
+
+## Launching a Shell in the Container
+
+A bash shell can be launched in the development container using
+
+```
+pipenv run devsh
+```
+
+This creates a new container which will get deleted once the shell session ends.
+
+It's possible to run a command directly; it supports the same arguments that `bash` supports.
+
+```bash
+pipenv run devsh -c 'echo hello'
+```
+
+### Invoking NsJail
+
+NsJail can be invoked in a more direct manner that does not require using a web server or its API. See `python -m snekbox --help`. Example usage:
+
+```bash
+python -m snekbox 'print("hello world!")' --time_limit 0
+```
+
+With this command, NsJail uses the same configuration normally used through the web API. It also has an alias, `pipenv run eval`.
+
+## Updating NsJail
+
+Updating NsJail mainly involves two steps:
+
+1. Change the version used by the `git clone` command in the [Dockerfile]
+2. Use `pipenv run protoc` to generate new Python code from the config protobuf
+
+Other things to look out for are breaking changes to NsJail's config format, its command-line interface, or its logging format. Additionally, dependencies may have to be adjusted in the Dockerfile to get a new version to build or run.
+
+[pipenv]: https://docs.pipenv.org/en/latest/
+[readme]: README.md
+[Dockerfile]: Dockerfile
diff --git a/Pipfile b/Pipfile
index 2e86b25..4a1e9d3 100644
--- a/Pipfile
+++ b/Pipfile
@@ -30,31 +30,24 @@ pydocstyle = "~= 5.1"
 python_version = "3.9"
 
 [scripts]
+eval = "python -m snekbox"
+webserver = "gunicorn -c config/gunicorn.conf.py snekbox.api.app"
+
+# Linting
 lint = "pre-commit run --all-files"
 precommit = "pre-commit install"
-test = "sh scripts/dev.sh -c 'pipenv run coverage run -m unittest'"
-testb = """
-    sh scripts/dev.sh \
-        --build \
-        --clean \
-        -c 'pipenv run coverage run -m unittest'
-"""
+
+# Testing
 report = "coverage report"
-snekbox = "gunicorn -c config/gunicorn.conf.py snekbox.api.app"
-eval = "python -m snekbox"
-devsh = "sh scripts/dev.sh"
-protoc = "sh scripts/protoc.sh"
-build = """
-    docker build \
-        -t ghcr.io/python-discord/snekbox:latest \
-        -f Dockerfile \
-        .
-"""
-builddev = """
-    docker build \
-        -t ghcr.io/python-discord/snekbox-venv:dev \
-        -f Dockerfile \
-        --target venv \
-        --build-arg DEV=1 \
-        .
+# Fix ownership of the coverage file even if tests fail & preserve exit code
+test = """
+    docker-compose run --entrypoint /bin/bash --rm snekbox -c \
+    'coverage run -m unittest; e=$?; chown --reference=. .coverage; exit $e'
 """
+
+# Docker
+build = "docker build -t ghcr.io/python-discord/snekbox:latest ."
+devsh = "docker-compose run --entrypoint /bin/bash --rm snekbox"
+
+# Other
+protoc = "python -m scripts.protoc"
diff --git a/README.md b/README.md
index 7e66c47..423cd50 100644
--- a/README.md
+++ b/README.md
@@ -25,7 +25,7 @@ result <- |             |<----------|           | <----------+
 
 ```
 
-The code is executed in a Python process that is launched through [NsJail], which is responsible for sandboxing the Python process. See [`snekbox.cfg`] for the NsJail configuration.
+The code is executed in a Python process that is launched through [NsJail], which is responsible for sandboxing the Python process.
 
 The output returned by snekbox is truncated at around 1 MB.
 
@@ -47,120 +47,62 @@ To run it in the background, use the `-d` option. See the documentation on [`doc
 
 The above command will make the API accessible on the host via `http://localhost:8060/`. Currently, there's only one endpoint: `http://localhost:8060/eval`.
 
-## Third-party Packages
-
-By default, the Python interpreter has no access to any packages besides the
-standard library. Even snekbox's own dependencies like Falcon and Gunicorn are
-not exposed.
-
-To expose third-party Python packages during evaluation, install them to a custom user site:
-
-```sh
-docker exec snekbox /bin/sh -c 'PYTHONUSERBASE=/snekbox/user_base pip install numpy'
-```
-
-In the above command, `snekbox` is the name of the running container. The name may be different and can be checked with `docker ps`.
-
-The packages will be installed to the user site within `/snekbox/user_base`. To persist the installed packages, a volume for the directory can be created with Docker. For an example, see [`docker-compose.yml`].
-
-If `pip`, `setuptools`, or `wheel` are dependencies or need to be exposed, then use the `--ignore-installed` option with pip. However, note that this will also re-install packages present in the custom user site, effectively making caching it futile. Current limitations of pip don't allow it to ignore packages extant outside the installation destination.
-
-## Development Environment
-
-### Initial Setup
-
-A Python 3.9 interpreter and the [pipenv] package are required. Once those requirements are satisfied, install the project's dependencies:
-
-```
-pipenv sync --dev
-```
-
-Follow that up with setting up the pre-commit hook:
-
-```
-pipenv run precommit
-```
-
-Now Flake8 will run and lint staged changes whenever an attempt to commit the changes is made. Flake8 can still be invoked manually:
-
-```
-pipenv run lint
-```
-
-### Running snekbox
+## Configuration
 
-The Docker image can be built with:
+Configuration files can be edited directly. However, this requires rebuilding the image. Alternatively, a Docker volume or bind mounts can be used to override the configuration files at their default locations.
 
-```
-pipenv run build
-```
+### NsJail
 
-Use Docker Compose to start snekbox:
+The main features of the default configuration are:
 
-```
-docker-compose up
-```
+* Time limit
+* Memory limit
+* Process count limit
+* No networking
+* Restricted, read-only filesystem
 
-### Running Tests
+NsJail is configured through [`snekbox.cfg`]. It contains the exact values for the items listed above. The configuration format is defined by a [protobuf file][7] which can be referred to for documentation. The command-line options of NsJail can also serve as documentation since they closely follow the config file format.
 
-Tests are run through coverage.py using unittest. Before tests can run, the dev venv Docker image has to be built:
+### Gunicorn
 
-```
-pipenv run builddev
-```
+[Gunicorn settings] can be found in [`gunicorn.conf.py`]. In the default configuration, the worker count and the bind address are likely the only things of any interest. Since it uses the default synchronous workers, the [worker count] effectively determines how many concurrent code evaluations can be performed.
 
-Alternatively, the following command will build the image and then run the tests:
+### Environment Variables
 
-```
-pipenv run testb
-```
+All environment variables have defaults and are therefore not required to be set.
 
-If the image doesn't need to be built, the tests can be run with:
+Name | Description
+---- | -----------
+`DEBUG` | Enable debug logging if set to a non-empty value.
+`GIT_SHA` | [Sentry release] identifier. Set in CI.
+`NSJAIL_CFG` | Path to the NsJail configuration file.
+`NSJAIL_PATH` | Path to the NsJail binary.
+`SNEKBOX_SENTRY_DSN` | [Data Source Name] for Sentry. Sentry is disabled if left unset.
 
-```
-pipenv run test
-```
-
-### Coverage
+Note: relative paths are relative to the root of the repository.
 
-To see a coverage report, run
+## Third-party Packages
 
-```
-pipenv run report
-```
+By default, the Python interpreter has no access to any packages besides the
+standard library. Even snekbox's own dependencies like Falcon and Gunicorn are
+not exposed.
 
-Alternatively, a report can be generated as HTML:
+To expose third-party Python packages during evaluation, install them to a custom user site:
 
+```sh
+docker exec snekbox /bin/sh -c 'PYTHONUSERBASE=/snekbox/user_base pip install numpy'
 ```
-pipenv run coverage html
-```
-
-The HTML will output to `./htmlcov/` by default
-
-
-### The `devsh` Helper Script
-
-This script starts a `bash` shell inside the venv Docker container and attaches to it. Unlike the production image, the venv image that is built by this script contains dev dependencies too. The project directory is mounted inside the container so any filesystem changes made inside the container affect the actual local project.
 
-#### Usage
-
-```
-pipenv run devsh [--build [--clean]] [bash_args ...]
-```
+In the above command, `snekbox` is the name of the running container. The name may be different and can be checked with `docker ps`.
 
-* `--build` Build the venv Docker image
-* `--clean` Clean up dangling Docker images (only works if `--build` precedes it)
-* `bash_args` Arguments to pass to `/bin/bash` (for example `-c "echo hello"`). An interactive shell is launched if no arguments are given
+The packages will be installed to the user site within `/snekbox/user_base`. To persist the installed packages, a volume for the directory can be created with Docker. For an example, see [`docker-compose.yml`].
 
-#### Invoking NsJail
+If `pip`, `setuptools`, or `wheel` are dependencies or need to be exposed, then use the `--ignore-installed` option with pip. However, note that this will also re-install packages present in the custom user site, effectively making caching it futile. Current limitations of pip don't allow it to ignore packages extant outside the installation destination.
 
-NsJail can be invoked in a more direct manner that does not require using a web server or its API. See `python -m snekbox --help`. Example usage:
+## Development Environment
 
-```bash
-python -m snekbox 'print("hello world!")' --time_limit 0
-```
+See [DEVELOPING.md](DEVELOPING.md).
 
-With this command, NsJail uses the same configuration normally used through the web API. It also has an alias, `pipenv run eval`.
 
 [1]: https://github.com/python-discord/snekbox/workflows/Lint,%20Test,%20Build,%20Push/badge.svg?branch=master
 [2]: https://github.com/python-discord/snekbox/actions?query=workflow%3A%22Lint%2C+Test%2C+Build%2C+Push%22+branch%3Amaster
@@ -168,6 +110,8 @@ With this command, NsJail uses the same configuration normally used through the
 [4]: https://coveralls.io/github/python-discord/snekbox?branch=master
 [5]: https://raw.githubusercontent.com/python-discord/branding/master/logos/badge/badge_github.svg
 [6]: https://discord.gg/python
+[7]: https://github.com/google/nsjail/blob/master/config.proto
+[`gunicorn.conf.py`]: config/gunicorn.conf.py
 [`snekbox.cfg`]: config/snekbox.cfg
 [`snekapi.py`]: snekbox/api/snekapi.py
 [`resources`]: snekbox/api/resources
@@ -176,5 +120,8 @@ With this command, NsJail uses the same configuration normally used through the
 [nsjail]: https://github.com/google/nsjail
 [falcon]: https://falconframework.org/
 [gunicorn]: https://gunicorn.org/
+[gunicorn settings]: https://docs.gunicorn.org/en/latest/settings.html
+[worker count]: https://docs.gunicorn.org/en/latest/design.html#how-many-workers
+[sentry release]: https://docs.sentry.io/platforms/python/configuration/releases/
+[data source name]: https://docs.sentry.io/product/sentry-basics/dsn-explainer/
 [GitHub Container Registry]: https://github.com/orgs/python-discord/packages/container/package/snekbox
-[pipenv]: https://docs.pipenv.org/en/latest/
diff --git a/docker-compose.yml b/docker-compose.yml
index a7747a6..f546024 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -2,17 +2,29 @@ version: "3.7"
 
 services:
   snekbox:
-    container_name: snekbox
+    container_name: snekbox_dev
+    hostname: snekbox_dev
     privileged: true
-    image: ghcr.io/python-discord/snekbox:latest
+    image: ghcr.io/python-discord/snekbox${IMAGE_SUFFIX:-:dev}
     ports:
      - 8060:8060
     init: true
     ipc: none
+    tty: true
+    working_dir: $PWD
+    environment:
+      DEBUG: 1
+      PIPENV_PIPFILE: /snekbox/Pipfile
+      PYTHONDONTWRITEBYTECODE: 1
     build:
       context: .
       dockerfile: Dockerfile
+      args:
+        DEV: 1
+      cache_from:
+        - ghcr.io/python-discord/snekbox:latest
     volumes:
+      - $PWD:$PWD
       - user-base:/snekbox/user_base
 
 volumes:
diff --git a/scripts/dev.sh b/scripts/dev.sh
deleted file mode 100755
index efbd93a..0000000
--- a/scripts/dev.sh
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/env sh
-
-# Sets up a development environment and runs a shell in a docker container.
-# Usage: dev.sh [--build [--clean]] [bash_args ...]
-
-if [ "$1" = "--build" ]; then
-    shift
-    printf "Building ghcr.io/python-discord/snekbox-venv:dev..."
-
-    docker build \
-        -t ghcr.io/python-discord/snekbox-venv:dev \
-        -f Dockerfile \
-        --build-arg DEV=1 \
-        --target venv \
-        -q \
-        . \
-        >/dev/null \
-    && printf " done!\n" || exit "$?"
-
-    if [ "$1" = "--clean" ]; then
-        shift
-        dangling_imgs=$(docker images -f "dangling=true" -q)
-
-        if [ -n "${dangling_imgs}" ]; then
-            printf "Removing dangling images..."
-
-            # shellcheck disable=SC2086
-            docker rmi $dangling_imgs >/dev/null \
-            && printf " done!\n" || exit "$?"
-        fi
-    fi
-fi
-
-# Keep the container up in the background so it doesn't have to be restarted
-# for the ownership fix.
-# The volume is mounted to same the path in the container as the source
-# directory on the host to ensure coverage can find the source files.
-docker run \
-    --tty \
-    --detach \
-    --name snekbox_test \
-    --privileged \
-    --hostname pdsnk-dev \
-    --ipc="none" \
-    -e PYTHONDONTWRITEBYTECODE=1 \
-    -e PIPENV_PIPFILE="/snekbox/Pipfile" \
-    --volume "${PWD}":"${PWD}" \
-    --workdir "${PWD}"\
-    --entrypoint /bin/bash \
-    ghcr.io/python-discord/snekbox-venv:dev \
-    >/dev/null \
-
-# Execute the given command(s)
-docker exec -it snekbox_test /bin/bash "$@"
-
-# Fix ownership of coverage file
-# BusyBox doesn't support --reference for chown
-docker exec \
-    -it \
-    -e CWD="${PWD}" \
-    snekbox_test \
-    /bin/bash \
-    -c 'chown "$(stat -c "%u:%g" "${CWD}")" "${CWD}/.coverage"'
-
-docker rm -f snekbox_test >/dev/null # Stop and remove the container
diff --git a/scripts/protoc.py b/scripts/protoc.py
new file mode 100755
index 0000000..09429d3
--- /dev/null
+++ b/scripts/protoc.py
@@ -0,0 +1,62 @@
+#!/usr/bin/env python3
+import shutil
+import subprocess
+import sys
+from argparse import ArgumentParser
+from pathlib import Path
+from tempfile import TemporaryDirectory
+from urllib.request import urlopen
+
+SRC_DIR = Path("snekbox").resolve(strict=True)
+FILE_NAME = "config"
+
+
+def compile_proto(path: Path) -> None:
+    """Compile a protobuf file at `path` into Python code."""
+    protoc_bin = shutil.which("protoc")
+    if not protoc_bin:
+        print("protoc binary could not be found on PATH", file=sys.stderr)
+        sys.exit(1)
+
+    args = [protoc_bin, f"--proto_path={path.parent}", f"--python_out={SRC_DIR}", path]
+    result = subprocess.run(args)
+
+    if result.returncode != 0:
+        sys.exit(result.returncode)
+
+
+def get_version() -> str:
+    """Get the NsJail version from the command line arguments."""
+    parser = ArgumentParser(description="Compile an NsJail config protobuf into Python.")
+    parser.add_argument("version", help="the NsJail version from which to get the protobuf file")
+    args = parser.parse_args()
+
+    return args.version
+
+
+def main() -> None:
+    """Get a config.proto for NsJail and compile it into Python."""
+    version = get_version()
+    url = f"https://raw.githubusercontent.com/google/nsjail/{version}/config.proto"
+
+    with urlopen(url) as response:
+        if response.status >= 400:
+            print(f"Failed to retrieve config.proto: status {response.status}", file=sys.stderr)
+            sys.exit(1)
+
+        with TemporaryDirectory() as dir_name:
+            file_path = Path(dir_name) / f"{FILE_NAME}.proto"
+            with open(file_path, "wb") as file:
+                file.write(response.read())
+            compile_proto(file_path)
+
+    # Remove the _pb suffix from the generated Python file.
+    if generated_py := next(SRC_DIR.glob(f"{FILE_NAME}_pb*.py"), None):
+        generated_py.rename(generated_py.with_stem(FILE_NAME))
+    else:
+        print(f"Could not find the generated Python file in {SRC_DIR}.", file=sys.stderr)
+        sys.exit(1)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/scripts/protoc.sh b/scripts/protoc.sh
deleted file mode 100755
index 5771b95..0000000
--- a/scripts/protoc.sh
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/usr/bin/env sh
-
-set -eu
-
-URL='https://raw.githubusercontent.com/google/nsjail/2.9/config.proto'
-SRC_DIR='snekbox'
-FILE_NAME='config'
-PROTO_PATH="${SRC_DIR}/${FILE_NAME}.proto"
-
-curl -SsL "${URL}" -o "${PROTO_PATH}"
-protoc --proto_path="${SRC_DIR}" --python_out="${SRC_DIR}" "${PROTO_PATH}"
-
-rm -f "${PROTO_PATH}"
-mv -f "${SRC_DIR}/${FILE_NAME}_pb"*.py "${SRC_DIR}/${FILE_NAME}.py"