Skip to content

fix(aws): gate Fargate ALB traffic admission on /ready #1025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
IAmJSD wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(aws): gate Fargate ALB traffic admission on /ready #1025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b775945
fix(aws): gate Fargate ALB traffic admission on /ready
IAmJSD Oct 16, 2026
14e957f
fix(aws): reject redirects in the readiness gate and gate on the serv…
IAmJSD Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 56 additions & 53 deletions .github/workflows/deploy-aws-fargate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ jobs:
cancel-in-progress: false
outputs:
alb_url: ${{ steps.outputs.outputs.alb_url }}
app_url: ${{ steps.outputs.outputs.app_url }}
service_name: ${{ steps.outputs.outputs.service_name }}
# Set at job level, not workflow level: workflow-level `env:` is resolved
# before this job's `environment:` is in scope, so an environment-scoped
Expand Down Expand Up @@ -223,8 +224,13 @@ jobs:
id: outputs
run: |
cd terraform/environments/aws
echo "alb_url=$(terraform output -raw fargate_alb_dns_name 2>/dev/null | grep -v '::' || echo "")" >> "$GITHUB_OUTPUT"
echo "service_name=$(terraform output -raw fargate_service_name 2>/dev/null | grep -v '::' || echo "")" >> "$GITHUB_OUTPUT"
{
echo "alb_url=$(terraform output -raw fargate_alb_dns_name 2>/dev/null | grep -v '::' || echo "")"
# The URL that actually serves the app (https on the api-<env>
# alias when HTTPS is enabled), for the test-deployment gates (#647).
echo "app_url=$(terraform output -raw fargate_app_url 2>/dev/null | grep -v '::' || echo "")"
echo "service_name=$(terraform output -raw fargate_service_name 2>/dev/null | grep -v '::' || echo "")"
} >> "$GITHUB_OUTPUT"

- name: Save deployment info
env:
Expand Down Expand Up @@ -265,69 +271,66 @@ jobs:
contents: read

steps:
- name: Get ALB URL
# Gate on the URL that serves the app, not the bare ALB DNS name: with
# HTTPS enabled the port 80 listener answers 301 on every path, so an
# http://<ALB> gate would never reach a task (#647). fargate_app_url is
# https on the api-<env> alias then (the name the listener certificate
# covers), otherwise http on the ALB DNS name.
- name: Get app URL
id: get-url
env:
APP_URL: ${{ needs.deploy.outputs.app_url }}
run: |
ALB_URL="${{ needs.deploy.outputs.alb_url }}"
if [ -z "$ALB_URL" ]; then
echo "Failed to get ALB URL from deploy outputs"
if [ -z "$APP_URL" ]; then
echo "Failed to get the app URL (fargate_app_url) from deploy outputs"
exit 1
fi
# Ensure URL has scheme and no trailing slash
case "$ALB_URL" in
http://*|https://*) ;; # already has scheme
*) ALB_URL="http://$ALB_URL" ;;
case "$APP_URL" in
http://*|https://*) ;;
*) echo "fargate_app_url has no scheme: $APP_URL"; exit 1 ;;
esac
ALB_URL="${ALB_URL%/}"
echo "url=$ALB_URL" >> "$GITHUB_OUTPUT"
APP_URL="${APP_URL%/}"
echo "Gating on $APP_URL"
echo "url=$APP_URL" >> "$GITHUB_OUTPUT"

- name: Wait for deployment
run: |
echo "Waiting 60 seconds for ECS service to stabilize..."
sleep 60

- name: Test health endpoint
run: |
URL="${{ steps.get-url.outputs.url }}"
echo "Testing health endpoint: $URL/health"

for i in {1..10}; do
# /health reports but never triggers the lazy DB init on the HTTP
# server; any /api/ request does. /health below is the gate.
curl -s -o /dev/null --max-time 60 "$URL/api/auth/check-admin" || true
# /health returns HTTP 200 even when degraded; the body is the verdict.
if RESPONSE=$(curl -fsS --max-time 10 "$URL/health"); then
echo "Response: $RESPONSE"
if jq -e '.status == "healthy"' > /dev/null <<< "$RESPONSE"; then
echo "Health check passed!"
exit 0
fi
fi
echo "Attempt $i failed, retrying in 10 seconds..."
sleep 10
done

echo "Health check failed after 10 attempts"
exit 1

# Needs the repo for scripts/wait-for-healthy.sh, used by both gates below.
- name: Checkout code
uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
with:
persist-credentials: false
sparse-checkout: scripts/wait-for-healthy.sh

# Fast fail gate: the server initializes its database in the background
# at startup, so no warm-up request is needed to make progress (#488).
# /ready answers 503 while the task that answers is still initializing,
# which the script counts as a failed attempt (#647). 30 x 10s covers the
# 300s ECS health check grace period, so a slow first boot that ECS
# still tolerates does not fail this gate.
- name: Test readiness endpoint
env:
APP_URL: ${{ steps.get-url.outputs.url }}
HEALTH_REQUIRED_STREAK: "1"
HEALTH_MAX_ATTEMPTS: "30"
HEALTH_INTERVAL_SECONDS: "10"
run: bash scripts/wait-for-healthy.sh "$APP_URL"

# Rollout race (#513): during a rolling deploy the ALB can route a probe
# to a different task than the previous one, so one not-ready response
# must not fail the step. The script requires HEALTH_REQUIRED_STREAK
# consecutive ready responses within HEALTH_MAX_ATTEMPTS rounds and
# fails with the last body otherwise.
- name: Run smoke tests
run: |
URL="${{ steps.get-url.outputs.url }}"

echo "Running basic smoke tests..."

for i in {1..3}; do
if RESPONSE=$(curl -fsS --max-time 10 "$URL/health") &&
jq -e '.status == "healthy"' > /dev/null <<< "$RESPONSE"; then
echo "Health check $i: passed"
else
echo "Health check $i: failed: ${RESPONSE:-no response}"
exit 1
fi
sleep 2
done

echo "All smoke tests passed!"
env:
APP_URL: ${{ steps.get-url.outputs.url }}
HEALTH_REQUIRED_STREAK: "3"
HEALTH_MAX_ATTEMPTS: "30"
HEALTH_INTERVAL_SECONDS: "5"
run: bash scripts/wait-for-healthy.sh "$APP_URL"

# Summary
summary:
Expand Down
91 changes: 74 additions & 17 deletions scripts/test-wait-for-healthy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,35 @@ mkdir -p "$WORK/bin"
# H = HTTP 200 healthy
# D = HTTP 200 but degraded body: what /health returned pre-#488, which the
# pre-fix loop caught only by parsing the body
# U = HTTP 503 with the degraded body: what /ready returns, and what
# --fail-with-body still prints
# U = HTTP 503 with the degraded body: what /ready returns
# M = HTTP 301 to https://app.example/ready with an empty body: what an
# http:// URL gets from an HTTPS-only ALB listener (#647)
# R = connection refused
# Models the real curl flags the callers use: -o FILE, -w with %{http_code} and
# %{redirect_url}, -f/--fail (exit 22 on >= 400, no body) and --fail-with-body
# (exit 22 on >= 400, body still written). Without either, curl exits 0 for any
# HTTP status, including 3xx and 5xx.
cat > "$WORK/bin/curl" <<'STUB'
#!/usr/bin/env bash
fwb=0
fail=0
out=""
wfmt=""
prev=""
for a in "$@"; do
url="$a"
[ "$a" = "--fail-with-body" ] && fwb=1
# -f/--fail, alone or inside a short cluster such as -fsS.
case "$prev" in
-o) out="$a"; prev=""; continue ;;
-w) wfmt="$a"; prev=""; continue ;;
esac
case "$a" in
--*) [ "$a" = "--fail" ] && fail=1 ;;
-o|-w) prev="$a"; continue ;;
--fail-with-body) fwb=1 ;;
--fail) fail=1 ;;
--*) ;;
# -f inside a short cluster such as -fsS.
-*f*) fail=1 ;;
esac
url="$a"
done
# Real curl (7.76+) rejects -f/--fail together with --fail-with-body.
if [ "$fwb" = 1 ] && [ "$fail" = 1 ]; then
Expand All @@ -51,15 +65,33 @@ tokens=($SCENARIO)
i=$n
((i >= ${#tokens[@]})) && i=$((${#tokens[@]} - 1))
echo $((n + 1)) > "$STATE"
degraded='{"status":"degraded","checks":{"auth_store":{"status":"unhealthy","message":"auth store not initialized"}}}'
location=""
case "${tokens[$i]}" in
H) echo '{"status":"healthy","checks":{}}' ;;
D) echo '{"status":"degraded","checks":{"auth_store":{"status":"unhealthy","message":"auth store not initialized"}}}' ;;
# Real curl only prints the body on a 503 when --fail-with-body was passed;
# modeling that keeps the failure-diagnostics coverage honest.
U) [ "$fwb" = 1 ] && echo '{"status":"degraded","checks":{"auth_store":{"status":"unhealthy","message":"auth store not initialized"}}}'
echo "curl: (22) The requested URL returned error: 503" >&2; exit 22 ;;
R) echo "curl: (7) Failed to connect" >&2; exit 7 ;;
H) code=200; body='{"status":"healthy","checks":{}}' ;;
D) code=200; body="$degraded" ;;
U) code=503; body="$degraded" ;;
M) code=301; body=""; location="https://app.example/ready" ;;
R) echo "curl: (7) Failed to connect" >&2
[ -n "$wfmt" ] && printf '000 '
exit 7 ;;
esac
if ((code >= 400)) && [ "$fail" = 1 ]; then
echo "curl: (22) The requested URL returned error: $code" >&2
exit 22
fi
if [ -n "$body" ]; then
if [ -n "$out" ]; then echo "$body" > "$out"; else echo "$body"; fi
fi
if [ -n "$wfmt" ]; then
w="${wfmt//'%{http_code}'/$code}"
printf '%s' "${w//'%{redirect_url}'/$location}"
fi
if ((code >= 400)) && [ "$fwb" = 1 ]; then
echo "curl: (22) The requested URL returned error: $code" >&2
exit 22
fi
exit 0
STUB
chmod +x "$WORK/bin/curl"

Expand Down Expand Up @@ -94,6 +126,12 @@ run_case "connection refused forever fails with curl error" 1 "R" "Failed to con

run_case "curl error mid-streak resets it (H R H H U, streak 3, must not pass)" 1 "H R H H U" "Failed to connect"

# #647: an http:// URL in front of an HTTPS-only listener answers 301 on every
# path. Counting that as ready made the gate pass without reaching the app.
run_case "301 redirect forever fails and names the redirect target" 1 "M" "HTTP 301 redirect to https://app.example/ready"
run_case "301 mid-streak resets it (H H M H H U, must not pass)" 1 "H H M H H U" "auth store not initialized"
run_case "503 failure keeps the status and the body" 1 "U" "HTTP 503: {\"status\":\"degraded\""

# run_env_case LABEL EXPECTED_EXIT SCENARIO OUTPUT_SUBSTRING [ENV=VAL ...]
# Runs the SUT with ONLY the given env overrides (plus a zero sleep interval),
# so the script's own defaults (streak 3, 30 attempts) are exercised.
Expand Down Expand Up @@ -166,12 +204,19 @@ run_case "rollout race H U H H H passes under the readiness gate" 0 "H U H H H"
# flag combinations that the installed curl rejects. Skipped when python3 is
# missing; curl older than 7.76 has no --fail-with-body and would fail here.
if command -v python3 > /dev/null; then
# Two servers: one always answers 200, the other 503 with a degraded body.
# Three servers: one always answers 200, one 503 with a degraded body, and one
# 301 to https on every path (an HTTPS-only ALB's port 80 listener, #647).
python3 - > "$WORK/port" <<'PY' &
import http.server, sys, threading

class Handler(http.server.BaseHTTPRequestHandler):
def do_GET(self):
if self.server.mode == "redirect":
self.send_response(301)
self.send_header("Location", "https://app.example.invalid" + self.path)
self.send_header("Content-Length", "0")
self.end_headers()
return
code = 200 if self.server.mode == "ok" else 503
body = b'{"status":"degraded","checks":{"database":{"status":"unhealthy"}}}' if code == 503 else b'{"status":"healthy","checks":{}}'
self.send_response(code)
Expand All @@ -182,19 +227,19 @@ class Handler(http.server.BaseHTTPRequestHandler):
pass

servers = []
for mode in ("ok", "unready"):
for mode in ("ok", "unready", "redirect"):
s = http.server.HTTPServer(("127.0.0.1", 0), Handler)
s.mode = mode
servers.append(s)
print(servers[0].server_port, servers[1].server_port, flush=True)
print(servers[0].server_port, servers[1].server_port, servers[2].server_port, flush=True)
for s in servers[1:]:
threading.Thread(target=s.serve_forever, daemon=True).start()
servers[0].serve_forever()
PY
SRV_PID=$!
trap 'kill "$SRV_PID" 2>/dev/null || true; wait "$SRV_PID" 2>/dev/null || true; rm -rf "$WORK"' EXIT
for _ in {1..50}; do [[ -s "$WORK/port" ]] && break; sleep 0.1; done
read -r OK_PORT UNREADY_PORT < "$WORK/port"
read -r OK_PORT UNREADY_PORT REDIRECT_PORT < "$WORK/port"

out=$(HEALTH_INTERVAL_SECONDS=0 HEALTH_MAX_ATTEMPTS=3 HEALTH_REQUIRED_STREAK=3 \
"$SUT" "http://127.0.0.1:${OK_PORT}" 2>&1) && real_ok=0 || real_ok=$?
Expand All @@ -215,6 +260,18 @@ PY
echo "FAIL: real curl against a 503 /ready (exit $real_503)"; echo "$out"
((fail++)) || true
fi

# The reviewer's repro on #1025: a server answering 301 on every path must
# NOT pass, and the failure must name where it redirected to.
out=$(HEALTH_INTERVAL_SECONDS=0 HEALTH_MAX_ATTEMPTS=3 HEALTH_REQUIRED_STREAK=3 \
"$SUT" "http://127.0.0.1:${REDIRECT_PORT}" 2>&1) && real_301=0 || real_301=$?
if [[ "$real_301" -eq 1 && "$out" == *"HTTP 301 redirect to https://app.example.invalid/ready"* ]]; then
echo "PASS: real curl rejects a 301 /ready and names the redirect target"
((pass++)) || true
else
echo "FAIL: real curl against a 301 /ready (exit $real_301)"; echo "$out"
((fail++)) || true
fi
fi

echo "passed=$pass failed=$fail"
Expand Down
70 changes: 47 additions & 23 deletions scripts/wait-for-healthy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
#!/usr/bin/env bash
# wait-for-healthy.sh URL
#
# Smoke gate for a just-deployed service. Polls URL/ready until curl
# --fail-with-body accepts the response (any status below 400)
# HEALTH_REQUIRED_STREAK times in a row. A not-ready or failed response inside
# the polling budget resets the streak instead of failing, because during a
# rollout a probe can hit a replica that is still initializing. If the budget
# (HEALTH_MAX_ATTEMPTS) runs out before the streak is reached, exits 1 and
# prints the last response (or curl error).
# Smoke gate for a just-deployed service. Polls URL/ready until it answers a
# 2xx status HEALTH_REQUIRED_STREAK times in a row. A not-ready or failed
# response inside the polling budget resets the streak instead of failing,
# because during a rollout a probe can hit a replica that is still
# initializing. If the budget (HEALTH_MAX_ATTEMPTS) runs out before the streak
# is reached, exits 1 and prints the last response (or curl error).
#
# /ready is the traffic-admission endpoint: it answers 503 until the replica it
# answers for has completed initialization, so the gate reflects the same
# contract the load balancer uses (#488). /health answers 200 even when
# degraded and is a liveness signal only. No warm-up request is needed: the
# server initializes its database in the background at startup.
#
# Requires curl 7.76+ for --fail-with-body, so a 503 still prints the JSON
# check detail that explains which dependency is not ready. Do not add -f:
# curl rejects -f/--fail together with --fail-with-body.
# Only 2xx counts as ready. A 3xx is NOT ready and is logged with its redirect
# target: an http:// URL in front of an HTTPS-only listener answers 301 on every
# path, and counting that as ready would make the gate pass without ever
# reaching the app (#647). Redirects are deliberately not followed; point the
# gate at the URL that serves the app instead. A non-2xx body is kept so a 503
# still prints the JSON check detail that explains which dependency is not
# ready.
#
# Env (defaults suit the deploy workflows; tests shrink them):
# HEALTH_REQUIRED_STREAK consecutive ready responses needed (3)
Expand Down Expand Up @@ -47,22 +50,43 @@ fi
streak=0
last="no response"

tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT

for ((attempt = 1; attempt <= MAX_ATTEMPTS; attempt++)); do
# /ready answers 503 until the replica has completed initialization;
# --fail-with-body keeps the JSON check detail so a failure says WHICH
# dependency is not ready.
if body=$(curl -sS --fail-with-body --max-time 10 "${URL}/ready" 2>&1); then
streak=$((streak + 1))
echo "Readiness check ${attempt}: ready (${streak}/${REQUIRED_STREAK})"
if ((streak >= REQUIRED_STREAK)); then
echo "Service is ready."
exit 0
fi
# curl exits 0 for any HTTP status here (no -f), so the status is judged
# below. stderr goes to its own file: curl prints its -w output even when the
# request fails, and mixing the two would garble the error message.
: > "$tmp/body"
if meta=$(curl -sS --max-time 10 -o "$tmp/body" -w '%{http_code} %{redirect_url}' "${URL}/ready" 2> "$tmp/err"); then
code="${meta%% *}"
redirect="${meta#* }"
[[ "$redirect" == "$meta" ]] && redirect=""
body="$(cat "$tmp/body")"
case "$code" in
2??)
streak=$((streak + 1))
echo "Readiness check ${attempt}: ready (${streak}/${REQUIRED_STREAK})"
if ((streak >= REQUIRED_STREAK)); then
echo "Service is ready."
exit 0
fi
sleep "$INTERVAL"
continue
;;
3??)
last="HTTP ${code} redirect to ${redirect:-<no Location header>}; redirects are not followed, so point the gate at the URL that serves the app"
;;
*)
last="HTTP ${code}: ${body:-<empty body>}"
;;
esac
else
last="${body:-no response}"
streak=0
echo "Readiness check ${attempt}: not ready yet, streak reset: ${last}"
err="$(cat "$tmp/err")"
last="${err:-no response}"
fi
streak=0
echo "Readiness check ${attempt}: not ready yet, streak reset: ${last}"
sleep "$INTERVAL"
done

Expand Down
Loading
Loading