From 49a3ef2fc8873c581b20c98488ac336aee0e0aa9 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Sun, 6 Sep 2026 02:10:21 -0600 Subject: [PATCH 1/2] Add automated test for docker-entrypoint.sh docker-entrypoint.sh translates BACKUP_DIRECTORY, SCHEDULED_USERS and BACKUP_INTERVAL_MS into JVM system properties, but neither CI job executed it: `build` runs `mvn clean verify`, which cannot run a shell script, and `docker-build` only copies the script into the image without starting a container. Entrypoint changes therefore landed on a fully green CI run with the changed lines never having been run. docker-entrypoint-test.sh puts a stub `java` on PATH that prints its arguments wrapped in brackets, then asserts the argument list the entrypoint builds for seven cases: no variables set, each variable set individually, all three set together, all three set to the empty string (which must add no arguments), and a value containing a space (which must stay a single argument). The bracket delimiting is what makes the last case meaningful. The test runs as a step in the docker-build job, so it gates the same pull requests the image build already gates. Closes #13 Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/build.yml | 3 ++ CHANGELOG.md | 1 + CONTRIBUTING.md | 8 +++ README.md | 18 +++++++ docker-entrypoint-test.sh | 103 ++++++++++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+) create mode 100755 docker-entrypoint-test.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 72e25b2..cc5e339 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -32,6 +32,9 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Test entrypoint script + run: sh docker-entrypoint-test.sh + - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 105b98b..5eebcab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added - `BACKUP_INTERVAL_MS` environment variable for the Docker daemon image, mapped by `docker-entrypoint.sh` to `-Dbackup.scheduled.interval.ms`, so the backup interval can be configured from `.env`/`docker-compose.yml` without overriding the entrypoint +- `docker-entrypoint-test.sh`, a shell test that runs `docker-entrypoint.sh` against a stub `java` and asserts the argument list built for each combination of `BACKUP_DIRECTORY`, `SCHEDULED_USERS` and `BACKUP_INTERVAL_MS`, including empty values and values containing spaces; it runs in the `docker-build` CI job, so entrypoint changes are no longer merged unexecuted ### Fixed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7e58a2a..c4c4e06 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,6 +69,14 @@ Windows: mvn clean test ``` +The Docker entrypoint script is covered by a separate shell test, which runs `docker-entrypoint.sh` against a stub `java` and asserts the arguments it builds. It requires a POSIX shell, so run it on Linux / macOS (or under WSL or Git Bash on Windows): + +```bash +sh docker-entrypoint-test.sh +``` + +Both suites run in CI on every pull request. + ## Questions Ask in the [Discord server](https://discord.gg/xXtuAQ2). diff --git a/README.md b/README.md index a041406..16d962c 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,24 @@ mvn clean test If you see `BUILD SUCCESS`, the tests have passed. +### Entrypoint Script Test + +`docker-entrypoint.sh` is covered by a shell test that runs it against a stub `java` and asserts the arguments it builds. A POSIX shell is required, so on Windows it is run under WSL or Git Bash. + +Linux / macOS: + +```bash +sh docker-entrypoint-test.sh +``` + +Windows (Git Bash / WSL): + +```bash +sh docker-entrypoint-test.sh +``` + +If you see `All docker-entrypoint.sh tests passed.`, the tests have passed. + ## Development ### Building and Running Locally diff --git a/docker-entrypoint-test.sh b/docker-entrypoint-test.sh new file mode 100755 index 0000000..4480882 --- /dev/null +++ b/docker-entrypoint-test.sh @@ -0,0 +1,103 @@ +#!/bin/sh +# Tests docker-entrypoint.sh by putting a stub `java` on PATH that prints its +# arguments instead of starting a JVM, then asserting the argument list the +# entrypoint builds for each combination of the environment variables it reads. +# +# Each argument is printed wrapped in brackets, so a value containing a space is +# distinguishable from two separate arguments. +# +# Run from the repository root: +# sh docker-entrypoint-test.sh + +SCRIPT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +ENTRYPOINT="$SCRIPT_DIR/docker-entrypoint.sh" + +if [ ! -f "$ENTRYPOINT" ]; then + echo "docker-entrypoint.sh not found at $ENTRYPOINT" >&2 + exit 1 +fi + +STUB_BIN=$(mktemp -d) +trap 'rm -rf "$STUB_BIN"' EXIT INT TERM + +cat > "$STUB_BIN/java" <<'STUB' +#!/bin/sh +for arg in "$@"; do + printf '[%s]' "$arg" +done +echo +STUB +chmod +x "$STUB_BIN/java" + +PROFILE="[-Dspring.profiles.active=daemon]" +JAR="[-jar][gh-backup.jar]" + +failures=0 + +# Runs the entrypoint with only the given NAME=VALUE assignments exported, so a +# variable set in the caller's environment cannot leak into a case. Callers +# invoke this inside a command substitution, so the changes stay in a subshell. +run_entrypoint() { + unset BACKUP_DIRECTORY SCHEDULED_USERS BACKUP_INTERVAL_MS + for assignment in "$@"; do + name=${assignment%%=*} + value=${assignment#*=} + eval "export $name=\"\$value\"" + done + PATH="$STUB_BIN:$PATH" sh "$ENTRYPOINT" +} + +expect() { + description=$1 + expected=$2 + shift 2 + + actual=$(run_entrypoint "$@") + + if [ "$actual" = "$expected" ]; then + echo "PASS: $description" + else + echo "FAIL: $description" + echo " expected: $expected" + echo " actual: $actual" + failures=$((failures + 1)) + fi +} + +expect "no variables set" \ + "$PROFILE$JAR" + +expect "BACKUP_DIRECTORY only" \ + "$PROFILE[-Dbackup.directory=/data/backups]$JAR" \ + "BACKUP_DIRECTORY=/data/backups" + +expect "SCHEDULED_USERS only" \ + "$PROFILE[-Dbackup.scheduled.users=octocat,github]$JAR" \ + "SCHEDULED_USERS=octocat,github" + +expect "BACKUP_INTERVAL_MS only" \ + "$PROFILE[-Dbackup.scheduled.interval.ms=3600000]$JAR" \ + "BACKUP_INTERVAL_MS=3600000" + +expect "all variables set, in declaration order" \ + "$PROFILE[-Dbackup.directory=/backups][-Dbackup.scheduled.users=octocat][-Dbackup.scheduled.interval.ms=60000]$JAR" \ + "BACKUP_DIRECTORY=/backups" "SCHEDULED_USERS=octocat" "BACKUP_INTERVAL_MS=60000" + +# The Dockerfile ships GITHUB_TOKEN, SCHEDULED_USERS and BACKUP_INTERVAL_MS as +# empty defaults, so an empty value must be treated as unset rather than passed +# through as an empty system property. +expect "empty variables add no arguments" \ + "$PROFILE$JAR" \ + "BACKUP_DIRECTORY=" "SCHEDULED_USERS=" "BACKUP_INTERVAL_MS=" + +expect "value containing a space stays a single argument" \ + "$PROFILE[-Dbackup.directory=/data/my backups]$JAR" \ + "BACKUP_DIRECTORY=/data/my backups" + +if [ "$failures" -eq 0 ]; then + echo "All docker-entrypoint.sh tests passed." + exit 0 +fi + +echo "$failures docker-entrypoint.sh test(s) failed." >&2 +exit 1 From 24b846f6641f44355598fdb3777b48e06b4b3a65 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Sun, 6 Sep 2026 02:13:42 -0600 Subject: [PATCH 2/2] Replace eval with an explicit case in the entrypoint test The NAME=VALUE parsing used eval to export a dynamically-named variable, which would mis-handle a value containing a quote or backtick and, more importantly, silently accepted a misspelled variable name: a case written against SCHEDULEDUSERS would export a variable the entrypoint never reads and still assert the empty-variable argument list, looking like coverage it did not provide. A case statement over the three variables docker-entrypoint.sh actually reads removes the eval and fails loudly on any other name. The comment on the empty-value case also named GITHUB_TOKEN, which the entrypoint does not read; it is trimmed to the two variables the case covers. Co-authored-by: Claude Opus 5 (1M context) --- docker-entrypoint-test.sh | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/docker-entrypoint-test.sh b/docker-entrypoint-test.sh index 4480882..3e43b85 100755 --- a/docker-entrypoint-test.sh +++ b/docker-entrypoint-test.sh @@ -40,9 +40,24 @@ failures=0 run_entrypoint() { unset BACKUP_DIRECTORY SCHEDULED_USERS BACKUP_INTERVAL_MS for assignment in "$@"; do - name=${assignment%%=*} - value=${assignment#*=} - eval "export $name=\"\$value\"" + case $assignment in + BACKUP_DIRECTORY=*) + BACKUP_DIRECTORY=${assignment#*=} + export BACKUP_DIRECTORY + ;; + SCHEDULED_USERS=*) + SCHEDULED_USERS=${assignment#*=} + export SCHEDULED_USERS + ;; + BACKUP_INTERVAL_MS=*) + BACKUP_INTERVAL_MS=${assignment#*=} + export BACKUP_INTERVAL_MS + ;; + *) + echo "Not a variable docker-entrypoint.sh reads: $assignment" >&2 + exit 1 + ;; + esac done PATH="$STUB_BIN:$PATH" sh "$ENTRYPOINT" } @@ -83,9 +98,9 @@ expect "all variables set, in declaration order" \ "$PROFILE[-Dbackup.directory=/backups][-Dbackup.scheduled.users=octocat][-Dbackup.scheduled.interval.ms=60000]$JAR" \ "BACKUP_DIRECTORY=/backups" "SCHEDULED_USERS=octocat" "BACKUP_INTERVAL_MS=60000" -# The Dockerfile ships GITHUB_TOKEN, SCHEDULED_USERS and BACKUP_INTERVAL_MS as -# empty defaults, so an empty value must be treated as unset rather than passed -# through as an empty system property. +# The Dockerfile ships SCHEDULED_USERS and BACKUP_INTERVAL_MS as empty defaults, +# so an empty value must be treated as unset rather than passed through as an +# empty system property. expect "empty variables add no arguments" \ "$PROFILE$JAR" \ "BACKUP_DIRECTORY=" "SCHEDULED_USERS=" "BACKUP_INTERVAL_MS="