Skip to content

fix(deploy): second-round hardening — drain budget, CI smoke, SSL_REQUIRED trim#300

Merged
vrogojin merged 1 commit intomainfrom
fix/ssl-docker-hardening-round2
Apr 18, 2026
Merged

fix(deploy): second-round hardening — drain budget, CI smoke, SSL_REQUIRED trim#300
vrogojin merged 1 commit intomainfrom
fix/ssl-docker-hardening-round2

Conversation

@vrogojin
Copy link
Copy Markdown
Contributor

Summary

Second pass of the steelman review on the hardening landed via #298. All findings from that review that could be addressed with code are addressed here. Branch protection on `main` is the only outstanding item and needs a GitHub Settings UI click, not code.

entrypoint.sh

  • nginx drain budget 2s → 10s. The previous poll loop (`10 × sleep 0.2`) gave in-flight requests only 2 seconds to finish before tini SIGKILLed nginx. Matches nginx's own graceful-stop SLA now.
  • Log timeout. If the 10s budget is exceeded, an explicit `WARNING: nginx did not drain within 10s; forcing exit` hits stderr so operators see truncated requests with a reason rather than silence.
  • Integer sleep. Using `sleep 1` instead of `sleep 0.2` so the loop is portable to BusyBox-coreutils bases (`sleep 0.2` becomes `sleep 0` there, producing a spin loop).
  • SSL_REQUIRED whitespace trim. Values like `" true "` (common when pasted from `.env` files) are now accepted after ltrim/rtrim instead of hard-failing the container.

docker-validate.yml

  • `workflow_dispatch` trigger added so transient CI failures can be re-run without pushing an empty commit.
  • Smoke tests added for both matrix rows:
    • `Dockerfile`: `nginx -t` verifies the default config parses.
    • `Dockerfile.ssl`: confirms `tini`, `sphere-entrypoint`, `curl`, and `nginx` are present and that the entrypoint script is syntactically valid bash. Catches entrypoint-path typos and shebang regressions that previously only surfaced on deploy.
  • Switched `load: false` → `load: true` so the smoke steps can actually run the built image.

Test plan (ran locally)

  • `bash -n deploy/entrypoint.sh` clean.
  • `docker build -f Dockerfile.ssl` succeeds.
  • `SSL_REQUIRED=" true "` now boots (was: exit 1).
  • `SSL_REQUIRED="bogus"` still exits 1 with the clear error.
  • `docker stop --time=15` produces exit code 143 (signal-clean shutdown path).
  • Smoke command used in CI runs green against the locally-built image.
  • CI on this PR: `validate (Dockerfile)` and `validate (Dockerfile.ssl)` both green — gate for merge.

Outstanding (non-code)

  • Enable branch protection on `main` and list `validate (Dockerfile)` + `validate (Dockerfile.ssl)` as required status checks. Requires GitHub Settings → Branches UI.

…IRED trim)

Addresses the second steelman review of the #298 hardening.

entrypoint.sh:
- Extend nginx drain budget from 2s (10 x 0.2s) to 10s (10 x 1s) so
  legitimate in-flight requests (SSE, slow clients) finish before tini
  SIGKILLs nginx. Matches nginx's own graceful-stop SLA.
- Log a WARNING when nginx doesn't drain within the budget so operators
  see timed-out shutdowns instead of silently truncated requests.
- Integer-only `sleep` in the poll loop so the script is portable to
  BusyBox-coreutils bases if the runtime image ever migrates off Debian.
- Trim whitespace around SSL_REQUIRED before the case match so values
  like "  true  " (common paste-from-.env mistake) no longer hard-fail.

docker-validate.yml:
- Add `workflow_dispatch` trigger so flaky transient failures can be
  re-run without pushing an empty commit.
- Switch to `load: true` and add smoke test steps for both matrix rows:
    * Dockerfile: `nginx -t` catches default-config syntax regressions.
    * Dockerfile.ssl: verify tini + sphere-entrypoint + curl + nginx
      are present and the entrypoint script is syntactically valid
      bash. Catches entrypoint-path typos and shebang regressions that
      would otherwise only surface at deploy time.

Verified locally:
- docker build -f Dockerfile.ssl succeeds.
- SSL_REQUIRED="  true  " now starts cleanly (was: exit 1).
- SSL_REQUIRED="bogus" still exits 1 with a clear error.
- docker stop --time=15 still produces exit code 143.
- Smoke command used in CI works against the locally-built image.
@vrogojin vrogojin merged commit 06e6314 into main Apr 18, 2026
7 checks passed
@vrogojin vrogojin deleted the fix/ssl-docker-hardening-round2 branch April 18, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant