diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 0000000..2360e20 --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,37 @@ +name: Test + +on: + pull_request: + push: + branches: + - main + +jobs: + shell-tests: + name: entrypoint.sh tests + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Run api() retry tests (bash) + run: | + set -e + for t in tests/*.test.sh; do + echo "== $t (bash) ==" + bash "$t" + done + + # Exercise the real runtime shell. The Docker entrypoint runs the script + # with busybox `sh` in alpine:3.15.0 (see Dockerfile), which relies on ash + # extensions the ubuntu runner's /bin/sh (dash) does not support, so run + # the suite inside that image rather than under the host shell. + - name: Run api() retry tests (busybox sh, alpine:3.15.0) + run: | + docker run --rm -v "$PWD":/work -w /work \ + public.ecr.aws/docker/library/alpine:3.15.0 \ + sh -c 'apk add --no-cache bash curl jq coreutils >/dev/null && \ + set -e && \ + for t in tests/*.test.sh; do echo "== $t (busybox) =="; sh "$t"; done' diff --git a/entrypoint.sh b/entrypoint.sh index 7aa1808..8b6ede0 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -82,31 +82,73 @@ validate_args() { fi } -lets_wait() { - echo "Sleeping for ${wait_interval} seconds" - sleep "$wait_interval" -} - +# Perform a GitHub REST API call, retrying transient failures. +# +# GitHub occasionally returns transient errors on these endpoints - most notably +# the workflow-dispatch trigger, which intermittently 500s with the body +# {"message":"Failed to run workflow dispatch","status":"500"}. Network errors +# and HTTP 429/5xx are retried with capped exponential backoff. Other 4xx +# responses are client errors and fail fast, and once retries are exhausted the +# failure is still surfaced to the caller so a genuine outage is never masked. +# +# Tunable via environment (defaults in parentheses): +# API_MAX_ATTEMPTS total attempts before giving up (5) +# API_RETRY_BASE_SECONDS first backoff delay; doubles each retry (3) +# API_RETRY_MAX_SECONDS cap on any single backoff delay (30) api() { - path=$1; shift - if response=$(curl --fail-with-body -sSL \ - "${GITHUB_API_URL}/repos/${INPUT_OWNER}/${INPUT_REPO}/actions/$path" \ - -H "Authorization: Bearer ${INPUT_GITHUB_TOKEN}" \ - -H 'Accept: application/vnd.github.v3+json' \ - -H 'Content-Type: application/json' \ - "$@") - then - echo "$response" - else + local path=$1; shift + local max_attempts=${API_MAX_ATTEMPTS:-5} + local delay=${API_RETRY_BASE_SECONDS:-3} + local max_delay=${API_RETRY_MAX_SECONDS:-30} + local attempt=1 + local body_file http_code response + + while true; do + body_file=$(mktemp) + # No --fail-with-body: capture the status code via -w and branch on it, + # so we can tell a retryable 5xx apart from a fatal 4xx. A transport-level + # failure (curl exits non-zero, prints nothing) is mapped to code 000. + http_code=$(curl -sSL \ + -o "$body_file" \ + -w '%{http_code}' \ + "${GITHUB_API_URL}/repos/${INPUT_OWNER}/${INPUT_REPO}/actions/$path" \ + -H "Authorization: Bearer ${INPUT_GITHUB_TOKEN}" \ + -H 'Accept: application/vnd.github.v3+json' \ + -H 'Content-Type: application/json' \ + "$@") || http_code=000 + response=$(cat "$body_file") + rm -f "$body_file" + + # Success (2xx): return the response body. + if [ "$http_code" -ge 200 ] && [ "$http_code" -lt 300 ]; then + echo "$response" + return 0 + fi + + # Retry transport failures (000), rate limiting (429) and server errors (5xx). + # Retrying the non-idempotent dispatch POST assumes the failure was + # pre-creation - true for the observed "Failed to run workflow dispatch" 500. + # Even if a run were somehow created, trigger_workflow keys off newly-appeared + # runs and an ArgoCD sync is idempotent, so a duplicate is harmless. + if [ "$http_code" = "000" ] || [ "$http_code" = "429" ] || [ "$http_code" -ge 500 ]; then + if [ "$attempt" -lt "$max_attempts" ]; then + echo >&2 "api transient failure (HTTP ${http_code}) on ${path}; attempt ${attempt}/${max_attempts}, retrying in ${delay}s" + [ -n "$response" ] && echo >&2 "response: $response" + sleep "$delay" + attempt=$((attempt + 1)) + delay=$((delay * 2)) + [ "$delay" -gt "$max_delay" ] && delay=$max_delay + continue + fi + fi + + # Non-retryable error, or retries exhausted: surface the failure. echo >&2 "api failed:" echo >&2 "path: $path" + echo >&2 "http_code: $http_code" echo >&2 "response: $response" - if [[ "$response" == *'"Server Error"'* ]]; then - echo >&2 "Server error - trying again" - else - exit 1 - fi - fi + exit 1 + done } lets_wait() { @@ -242,4 +284,9 @@ main() { fi } -main +# Allow the script to be sourced by the test harness without executing main. +# TWAW_SOURCE_ONLY is never set in production (the Docker entrypoint runs the +# script directly), so the default behaviour is unchanged. +if [ "${TWAW_SOURCE_ONLY:-}" != "1" ]; then + main +fi diff --git a/tests/api_retry.test.sh b/tests/api_retry.test.sh new file mode 100755 index 0000000..4ea4ef2 --- /dev/null +++ b/tests/api_retry.test.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# Tests for the transient-error retry behaviour of entrypoint.sh's api() helper. +# Runs the real api() against a scripted `curl` stub (tests/stub/curl). +set -u + +HERE=$(cd "$(dirname "$0")" && pwd) +ROOT=$(cd "$HERE/.." && pwd) + +# Put the curl stub first on PATH; every other tool (jq, sed, mktemp) stays real. +export PATH="$HERE/stub:$PATH" + +export CURL_STUB_COUNTER +CURL_STUB_COUNTER=$(mktemp) +export CURL_STUB_SCENARIO +CURL_STUB_SCENARIO=$(mktemp) +trap 'rm -f "$CURL_STUB_COUNTER" "$CURL_STUB_SCENARIO"' EXIT + +# Values api() reads to build the request URL (contents irrelevant to the stub). +export INPUT_OWNER=cloudbeds INPUT_REPO=argocd-mfd INPUT_GITHUB_TOKEN=x +# Retry knobs: keep attempts small and eliminate real sleeping so tests are fast. +export API_MAX_ATTEMPTS=4 +export API_RETRY_BASE_SECONDS=0 + +# Source the script for its functions only; do not run main. +# shellcheck disable=SC1090 +TWAW_SOURCE_ONLY=1 . "$ROOT/entrypoint.sh" +set +e # entrypoint.sh enables `set -e`; disable it so the harness controls flow. + +fail=0 +pass() { printf 'ok - %s\n' "$1"; } +die() { printf 'FAIL - %s\n' "$1"; fail=1; } + +# scenario "" resets the counter and writes the response script. +scenario() { : > "$CURL_STUB_COUNTER"; printf '%b' "$1" > "$CURL_STUB_SCENARIO"; } +attempts() { cat "$CURL_STUB_COUNTER"; } + +S500='500\t{"message":"Failed to run workflow dispatch","status":"500"}' + +# --- Test 1: transient 500s then success -------------------------------------- +# GitHub's dispatch endpoint intermittently returns a 500 with this exact body +# (no "Server Error" string). api() must retry and ultimately succeed. +scenario "$S500\n$S500\n204\t\n" +# api() calls `exit` on hard failure, so always invoke it in a subshell. +out=$(api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null); rc=$? +if [ "$rc" -eq 0 ] && [ "$(attempts)" -eq 3 ]; then + pass "retries transient 500 then succeeds (3 attempts)" +else + die "retries transient 500 then succeeds: rc=$rc attempts=$(attempts) (want rc=0 attempts=3)" +fi + +# --- Test 2: genuine 4xx fails fast, no retry --------------------------------- +scenario '404\t{"message":"Not Found"}\n' +out=$(api "workflows/missing.yaml/dispatches" --data '{}' 2>/dev/null); rc=$? +if [ "$rc" -ne 0 ] && [ "$(attempts)" -eq 1 ]; then + pass "does not retry a 4xx (fails fast in 1 attempt)" +else + die "does not retry a 4xx: rc=$rc attempts=$(attempts) (want rc!=0 attempts=1)" +fi + +# --- Test 3: persistent 500 exhausts retries then fails ------------------------ +scenario "$S500\n$S500\n$S500\n$S500\n$S500\n" +out=$(api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null); rc=$? +if [ "$rc" -ne 0 ] && [ "$(attempts)" -eq "$API_MAX_ATTEMPTS" ]; then + pass "gives up after API_MAX_ATTEMPTS on persistent 500 (surfaces failure)" +else + die "persistent 500 exhausts retries: rc=$rc attempts=$(attempts) (want rc!=0 attempts=$API_MAX_ATTEMPTS)" +fi + +# --- Test 4: transport error (no HTTP response) is retried -------------------- +scenario 'NETERR\n200\t{"ok":true}\n' +if out=$(api "runs/123" 2>/dev/null); then rc=0; else rc=$?; fi +if [ "$rc" -eq 0 ] && [ "$(attempts)" -eq 2 ] && printf '%s' "$out" | grep -q '"ok":true'; then + pass "retries a transport failure then succeeds (2 attempts)" +else + die "retries a transport failure: rc=$rc attempts=$(attempts) out=$out" +fi + +# --- Test 5: a 429 (rate limit) is retried like a 5xx ------------------------- +scenario '429\t{"message":"API rate limit exceeded"}\n204\t\n' +out=$(api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null); rc=$? +if [ "$rc" -eq 0 ] && [ "$(attempts)" -eq 2 ]; then + pass "retries a 429 then succeeds (2 attempts)" +else + die "retries a 429: rc=$rc attempts=$(attempts) (want rc=0 attempts=2)" +fi + +# --- Test 6: backoff doubles each retry and caps at API_RETRY_MAX_SECONDS ------ +# The sleep stub records each requested delay (via SLEEP_LOG) instead of really +# sleeping, so the backoff sequence can be asserted without waiting. With base 3 +# and cap 10 over 5 persistent failures, the delays are 3, 6, 10 (12 capped), 10. +SLEEP_LOG=$(mktemp) +scenario "$S500\n$S500\n$S500\n$S500\n$S500\n" +out=$( + export SLEEP_LOG API_RETRY_BASE_SECONDS=3 API_RETRY_MAX_SECONDS=10 API_MAX_ATTEMPTS=5 + api "workflows/update-application.yaml/dispatches" --data '{}' 2>/dev/null +); rc=$? +seq=$(tr '\n' ' ' < "$SLEEP_LOG" | sed 's/ *$//') +rm -f "$SLEEP_LOG" +if [ "$rc" -ne 0 ] && [ "$(attempts)" -eq 5 ] && [ "$seq" = "3 6 10 10" ]; then + pass "backoff doubles then caps at API_RETRY_MAX_SECONDS (delays: 3 6 10 10)" +else + die "backoff sequence: rc=$rc attempts=$(attempts) delays=[$seq] (want attempts=5 delays='3 6 10 10')" +fi + +echo "----" +if [ "$fail" -eq 0 ]; then echo "ALL PASS"; else echo "SOME FAILED"; fi +exit "$fail" diff --git a/tests/api_retry_seterr.test.sh b/tests/api_retry_seterr.test.sh new file mode 100755 index 0000000..f0c4830 --- /dev/null +++ b/tests/api_retry_seterr.test.sh @@ -0,0 +1,39 @@ +#!/bin/sh +# Production runs entrypoint.sh with `set -e` (see the top of the script). The +# retry loop uses `A && B` lists and arithmetic that can trip `set -e` if written +# carelessly, so this test exercises the multi-retry path with `set -e` left ON +# and asserts the loop runs to completion instead of aborting early. +set -eu + +HERE=$(cd "$(dirname "$0")" && pwd) +ROOT=$(cd "$HERE/.." && pwd) +export PATH="$HERE/stub:$PATH" + +CURL_STUB_COUNTER=$(mktemp); export CURL_STUB_COUNTER +CURL_STUB_SCENARIO=$(mktemp); export CURL_STUB_SCENARIO +trap 'rm -f "$CURL_STUB_COUNTER" "$CURL_STUB_SCENARIO"' EXIT +export INPUT_OWNER=cloudbeds INPUT_REPO=argocd-mfd INPUT_GITHUB_TOKEN=x +export API_MAX_ATTEMPTS=4 API_RETRY_BASE_SECONDS=0 + +# Sourcing re-enables `set -e`; deliberately do NOT disable it afterwards. +TWAW_SOURCE_ONLY=1 . "$ROOT/entrypoint.sh" + +S500='500\t{"message":"Failed to run workflow dispatch","status":"500"}' +: > "$CURL_STUB_COUNTER" +printf '%b' "$S500\n$S500\n204\t\n" > "$CURL_STUB_SCENARIO" + +# api() calls exit on hard failure and inherits `set -e`; run it in a subshell +# inside an `if` so neither its exit nor a non-zero status aborts this harness. +if ( api "workflows/update-application.yaml/dispatches" --data '{}' >/dev/null 2>&1 ); then + rc=0 +else + rc=$? +fi +n=$(cat "$CURL_STUB_COUNTER") + +if [ "$rc" -eq 0 ] && [ "$n" -eq 3 ]; then + echo "ok - retry loop completes cleanly under set -e (3 attempts)" + exit 0 +fi +echo "FAIL - set -e retry path: rc=$rc attempts=$n (want rc=0 attempts=3)" +exit 1 diff --git a/tests/stub/curl b/tests/stub/curl new file mode 100755 index 0000000..79b25f5 --- /dev/null +++ b/tests/stub/curl @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# Faithful-enough `curl` stub for exercising entrypoint.sh's api() function. +# +# It understands only the flags api() actually uses (--fail-with-body, -o , +# -w ) and ignores everything else (-H, -sSL, --data, the URL). Behaviour is +# driven by two files so a single test can script a sequence of responses: +# +# $CURL_STUB_SCENARIO : one line per attempt. Either "" or the +# literal token NETERR to simulate a transport failure. +# $CURL_STUB_COUNTER : running attempt counter (created on first call). +# +# It emulates real curl closely enough that both the old (--fail-with-body, body +# on stdout) and the new (-o file + -w '%{http_code}') implementations work. +set -u + +fail_with_body=0 +out_file="" +w_fmt="" + +while [ $# -gt 0 ]; do + case "$1" in + --fail-with-body) fail_with_body=1 ;; + -o) shift; out_file="$1" ;; + -w) shift; w_fmt="$1" ;; + esac + shift +done + +n=$(cat "$CURL_STUB_COUNTER" 2>/dev/null || echo 0) +n=$((n + 1)) +echo "$n" > "$CURL_STUB_COUNTER" + +line=$(sed -n "${n}p" "$CURL_STUB_SCENARIO") +if [ "$line" = "NETERR" ] || [ -z "$line" ]; then + # Transport-level failure (connection reset, DNS, timeout): no HTTP response. + exit 7 +fi + +code=$(printf '%s' "$line" | cut -f1) +body=$(printf '%s' "$line" | cut -f2-) + +# Body goes to the -o file if requested, otherwise to stdout (like real curl). +if [ -n "$out_file" ]; then + printf '%s' "$body" > "$out_file" +else + printf '%s' "$body" +fi + +# The -w template is written to stdout with %{http_code} substituted. +if [ -n "$w_fmt" ]; then + printf '%s' "${w_fmt//%\{http_code\}/$code}" +fi + +# --fail-with-body makes real curl exit 22 on HTTP >= 400 (still emitting body). +if [ "$fail_with_body" -eq 1 ] && [ "$code" -ge 400 ]; then + exit 22 +fi +exit 0 diff --git a/tests/stub/sleep b/tests/stub/sleep new file mode 100755 index 0000000..ccee9d4 --- /dev/null +++ b/tests/stub/sleep @@ -0,0 +1,6 @@ +#!/bin/sh +# Test stub for `sleep`: records the requested duration (when SLEEP_LOG is set) +# and returns immediately instead of actually sleeping, so backoff timing can be +# asserted without real delays. When SLEEP_LOG is unset it is a silent no-op. +[ -n "${SLEEP_LOG:-}" ] && printf '%s\n' "$1" >> "$SLEEP_LOG" +exit 0