diff --git a/README.md b/README.md index d25714a..c1af4ff 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,23 @@ GitHub Action and Docker image used to deploy a Docker stack on a Docker Swarm. | `debug` | `DEBUG` | Verbose logging | | **0** | | `scale_after` | `SCALE_AFTER` | Scale a service after a deployment has converged successfully. Example: servicename=1 | | | +### A note on whitespace + +`registry`, `username`, `remote_host`, `remote_port` and `remote_user` cannot +contain whitespace, so any is removed before the value is used, and a line +naming the input is written to the log when that happens: + +``` +Input remote_host: removed whitespace from the value +``` + +This exists because a stray newline in a GitHub secret is invisible in the +repository UI, and used to surface much later as an opaque +`ssh: Could not resolve hostname`. + +`remote_private_key` and `password` are left exactly as given — newlines are +structural in a PEM key, and whitespace can be a legitimate part of a token. + ## Using the GitHub Action diff --git a/news/1.bugfix b/news/1.bugfix new file mode 100644 index 0000000..28e599d --- /dev/null +++ b/news/1.bugfix @@ -0,0 +1 @@ +Fixed a stray newline or space in `remote_host` (or `registry`, `username`, `remote_port`, `remote_user`) breaking the deploy with an opaque `ssh: Could not resolve hostname`. Whitespace is now removed from those inputs, and the log says when it was. @ericof diff --git a/scripts/docker-entrypoint.sh b/scripts/docker-entrypoint.sh index 12b42e7..1400f3d 100755 --- a/scripts/docker-entrypoint.sh +++ b/scripts/docker-entrypoint.sh @@ -10,6 +10,29 @@ ENV_FILE_PATH="/root/.env" OPTS=("--with-registry-auth" "--resolve-image=${RESOLVE_IMAGE:-always}") [ "${PRUNE:-0}" = "1" ] && OPTS+=("--prune") +# Inputs whose values cannot legally contain whitespace. A newline pasted into +# a GitHub secret is invisible in the repository UI and only surfaces much +# later, as an opaque `ssh: Could not resolve hostname`, so strip it here. +# +# REMOTE_PRIVATE_KEY and PASSWORD are deliberately NOT in this list, and must +# not be added: newlines are structural in a PEM key, and whitespace can be a +# legitimate part of a registry token. +TRIMMED_INPUTS=(REGISTRY USERNAME REMOTE_HOST REMOTE_PORT REMOTE_USER) + + +trim_inputs() { + local name original trimmed + for name in "${TRIMMED_INPUTS[@]}"; do + original="${!name-}" + trimmed="${original//[[:space:]]/}" + if [ "${trimmed}" != "${original}" ]; then + # Reported unconditionally, not only under DEBUG: the whole problem with + # this class of mistake is that it is invisible everywhere else. + echo "Input ${name,,}: removed whitespace from the value" + export "${name}=${trimmed}" + fi + done +} login() { echo "${PASSWORD}" | docker login "${REGISTRY}" -u "${USERNAME}" --password-stdin @@ -127,6 +150,12 @@ else SSH_VERBOSE="" fi +# NORMALISE INPUTS +# Runs before the first use of any of them, and before the required-input +# checks below, so an input that is nothing but whitespace is reported as +# missing rather than being passed on to ssh. +trim_inputs + # PROCEED WITH LOGIN if [ -z "${USERNAME}" ] || [ -z "${PASSWORD}" ]; then echo "Container Registry: No authentication provided" diff --git a/tests/entrypoint.bats b/tests/entrypoint.bats index cce23e6..a800341 100644 --- a/tests/entrypoint.bats +++ b/tests/entrypoint.bats @@ -80,6 +80,115 @@ run_entrypoint() { [[ "$output" == *"Input stack_name is required!"* ]] } +# --- input whitespace (issue #1) ---------------------------------------------- + +# Source the script, run trim_inputs against a controlled environment, and +# print the resulting value of one variable. +# +# The value is printed inside brackets on purpose: bats strips trailing +# newlines from $output, so an untrimmed `host\n` would compare equal to `host` +# and the assertion would pass whether the trimming happened or not. +trim_and_print() { + local var="$1" + shift + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" WANT="${var}" "$@" \ + bash -c " + source '${ENTRYPOINT}' + trim_inputs >/dev/null + printf '[%s]' \"\${!WANT}\" + " +} + +@test "remote_host loses a trailing newline (issue #1)" { + trim_and_print REMOTE_HOST REMOTE_HOST=$'swarm.example.com\n' + [ "$status" -eq 0 ] + [ "$output" = "[swarm.example.com]" ] +} + +@test "remote_host loses whitespace in the middle of the value (issue #1)" { + trim_and_print REMOTE_HOST REMOTE_HOST='swarm .example.com' + [ "$status" -eq 0 ] + [ "$output" = "[swarm.example.com]" ] +} + +@test "remote_user is trimmed (issue #1)" { + trim_and_print REMOTE_USER REMOTE_USER=$'deploy\n' + [ "$status" -eq 0 ] + [ "$output" = "[deploy]" ] +} + +@test "remote_port is trimmed (issue #1)" { + trim_and_print REMOTE_PORT REMOTE_PORT=$' 2222\n' + [ "$status" -eq 0 ] + [ "$output" = "[2222]" ] +} + +@test "registry is trimmed (issue #1)" { + trim_and_print REGISTRY REGISTRY=$'ghcr.io\n' + [ "$status" -eq 0 ] + [ "$output" = "[ghcr.io]" ] +} + +@test "username is trimmed (issue #1)" { + trim_and_print USERNAME USERNAME=$'someone\n' + [ "$status" -eq 0 ] + [ "$output" = "[someone]" ] +} + +@test "trimming an input is reported on stdout (issue #1)" { + # The point of the issue: the mistake is invisible everywhere else, so the + # fix has to be visible without DEBUG. + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \ + REMOTE_HOST=$'swarm.example.com\n' \ + bash -c "source '${ENTRYPOINT}'; trim_inputs" + [ "$status" -eq 0 ] + [[ "$output" == *"Input remote_host: removed whitespace from the value"* ]] +} + +@test "a clean input is left alone and reported silently (issue #1)" { + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \ + REMOTE_HOST=swarm.example.com REMOTE_USER=deploy REMOTE_PORT=22 \ + bash -c "source '${ENTRYPOINT}'; trim_inputs" + [ "$status" -eq 0 ] + [ -z "$output" ] +} + +@test "remote_private_key is never trimmed (issue #1)" { + # Newlines are structural in a PEM key. Counting them is what makes a + # regression here fail loudly instead of silently mangling the key. + run env -i PATH="${PATH}" HOME="${BATS_TEST_TMPDIR}" \ + REMOTE_PRIVATE_KEY=$'-----BEGIN-----\nabc\n-----END-----\n' \ + bash -c " + source '${ENTRYPOINT}' + trim_inputs >/dev/null + printf '%s' \"\${REMOTE_PRIVATE_KEY}\" | wc -l + " + [ "$status" -eq 0 ] + [ "${output// /}" = "3" ] +} + +@test "password is never trimmed (issue #1)" { + trim_and_print PASSWORD PASSWORD='tok en ' + [ "$status" -eq 0 ] + [ "$output" = "[tok en ]" ] +} + +@test "the trim runs as part of the deploy flow (issue #1)" { + # Every other test in this section calls trim_inputs directly; this one is + # what proves it is actually wired into the script's flow, and that it runs + # before the required-input checks. + run_entrypoint REMOTE_HOST=$'swarm.example.com\n' + [ "$status" -eq 1 ] + [[ "$output" == *"Input remote_host: removed whitespace from the value"* ]] + [[ "$output" == *"Input remote_user is required!"* ]] +} + +@test "a remote_host of only whitespace is reported as missing (issue #1)" { + run_entrypoint REMOTE_HOST=$' \n' + [ "$status" -eq 1 ] + [[ "$output" == *"Input remote_host is required!"* ]] +} + # --- container registry login ------------------------------------------------- @test "skips login when no credentials are given (issue #4)" {