Compare commits
6 Commits
issue-102-
...
c4022c0834
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c4022c0834 | ||
| be576096aa | |||
| 3941f0b0ff | |||
| 543005c0c2 | |||
| 9bfd033a29 | |||
| fd6397154a |
@@ -1,3 +1,6 @@
|
|||||||
|
# .ci-fingerprint is deliberately NOT excluded: it is the CI cache barrier
|
||||||
|
# that keeps the check stages from replaying a cached pass. See the lint
|
||||||
|
# stage of the Dockerfile.
|
||||||
.git/
|
.git/
|
||||||
bin/
|
bin/
|
||||||
*.md
|
*.md
|
||||||
|
|||||||
@@ -11,5 +11,53 @@ jobs:
|
|||||||
steps:
|
steps:
|
||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
||||||
|
with:
|
||||||
|
# The fingerprint step below needs history to find the last commit
|
||||||
|
# that touched the Docker build context.
|
||||||
|
fetch-depth: 0
|
||||||
|
|
||||||
|
- name: Neutralize superseded run statuses
|
||||||
|
# Gitea cancels the in-flight run when another commit is pushed to the
|
||||||
|
# same branch and records the cancellation as `failure`, so a commit
|
||||||
|
# that was never tested reads red. The cancellation is unconditional
|
||||||
|
# server-side for push events and cannot be disabled from a workflow
|
||||||
|
# file, so the superseding run rewrites those statuses to `skipped`.
|
||||||
|
# Only the exact cancellation status is touched; a real failure is
|
||||||
|
# left alone.
|
||||||
|
env:
|
||||||
|
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||||
|
run: |
|
||||||
|
set -eu
|
||||||
|
api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
|
||||||
|
ctx='check / check (push)'
|
||||||
|
for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
|
||||||
|
latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
|
||||||
|
--arg c "$ctx" \
|
||||||
|
'[.statuses[] | select(.context == $c)][0] // empty
|
||||||
|
| "\(.status)|\(.description)"')" || continue
|
||||||
|
[ "$latest" = 'failure|Has been cancelled' ] || continue
|
||||||
|
curl -sf -X POST "${api}/statuses/${sha}" \
|
||||||
|
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||||
|
-H 'Content-Type: application/json' \
|
||||||
|
-d "$(jq -nc --arg c "$ctx" '{
|
||||||
|
context: $c,
|
||||||
|
state: "skipped",
|
||||||
|
description: "Superseded by a newer commit; never tested"
|
||||||
|
}')" >/dev/null
|
||||||
|
echo "neutralized superseded status on ${sha}"
|
||||||
|
done
|
||||||
|
|
||||||
|
- name: Fingerprint the build context
|
||||||
|
# `.dockerignore` keeps docs out of the build context, so a docs-only
|
||||||
|
# commit legitimately replays the whole image from cache and stays
|
||||||
|
# cheap. Every other commit writes a new fingerprint into the context,
|
||||||
|
# which invalidates the `COPY . .` layer of both check stages: a
|
||||||
|
# commit that was never linted, formatted-checked, tested and built
|
||||||
|
# cannot report success from cache.
|
||||||
|
run: |
|
||||||
|
set -eu
|
||||||
|
fp="$(git log -1 --format=%H -- . ':!*.md' ':!LICENSE' ':!.editorconfig')"
|
||||||
|
printf '%s\n' "${fp:-$GITHUB_SHA}" > .ci-fingerprint
|
||||||
|
|
||||||
- name: Build Docker image (runs make check)
|
- name: Build Docker image (runs make check)
|
||||||
run: script/cibuild
|
run: script/cibuild
|
||||||
|
|||||||
3
.gitignore
vendored
3
.gitignore
vendored
@@ -42,3 +42,6 @@ data/
|
|||||||
# Temporary files
|
# Temporary files
|
||||||
tmp/
|
tmp/
|
||||||
temp/
|
temp/
|
||||||
|
|
||||||
|
# CI cache barrier, written into the build context by the check workflow
|
||||||
|
.ci-fingerprint
|
||||||
@@ -12,7 +12,11 @@ WORKDIR /src
|
|||||||
COPY go.mod go.sum ./
|
COPY go.mod go.sum ./
|
||||||
RUN go mod download
|
RUN go mod download
|
||||||
|
|
||||||
# Copy source code
|
# Copy source code. In CI the context also carries .ci-fingerprint, whose
|
||||||
|
# value changes with every commit that touches the build context (see
|
||||||
|
# .gitea/workflows/check.yml). That invalidates this layer, so the checks
|
||||||
|
# below cannot report success by replaying a cached pass. Do not add it to
|
||||||
|
# .dockerignore.
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
# Run formatting check and linter
|
# Run formatting check and linter
|
||||||
@@ -36,7 +40,8 @@ WORKDIR /build
|
|||||||
COPY go.mod go.sum ./
|
COPY go.mod go.sum ./
|
||||||
RUN go mod download
|
RUN go mod download
|
||||||
|
|
||||||
# Copy source code
|
# Copy source code, including the .ci-fingerprint cache barrier described in
|
||||||
|
# the lint stage above.
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
# Run tests and build
|
# Run tests and build
|
||||||
|
|||||||
54
README.md
54
README.md
@@ -93,6 +93,7 @@ TTY detection, and security headers are always applied.
|
|||||||
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
|
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
|
||||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
|
||||||
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
||||||
|
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
|
||||||
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
||||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
|
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` |
|
||||||
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
|
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) |
|
||||||
@@ -173,8 +174,12 @@ its value and refuses to start, rather than silently running with a
|
|||||||
substituted default. `PORT=eighty`, `DEBUG=ture`, and
|
substituted default. `PORT=eighty`, `DEBUG=ture`, and
|
||||||
`RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
|
`RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
|
||||||
additionally be a number in the range 1–65535,
|
additionally be a number in the range 1–65535,
|
||||||
`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
|
`RECEIVER_RATE_LIMIT` must be at least 1,
|
||||||
`TRUSTED_PROXIES` must be a CIDR block or a bare IP address.
|
`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
|
||||||
|
period, so `0s` or a negative value would crash the reaper after
|
||||||
|
startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or
|
||||||
|
a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a
|
||||||
|
non-positive value there means idle expiry is disabled, not invalid.
|
||||||
|
|
||||||
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
|
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
|
||||||
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
|
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
|
||||||
@@ -260,9 +265,10 @@ webhooker solves this by acting as a durable intermediary:
|
|||||||
targets simultaneously. This enables patterns like forwarding a
|
targets simultaneously. This enables patterns like forwarding a
|
||||||
GitHub webhook to both a deployment service and a Slack channel.
|
GitHub webhook to both a deployment service and a Slack channel.
|
||||||
|
|
||||||
5. **Replay** — Stored events can be manually redelivered for debugging
|
5. **Replay** (not yet implemented) — Every received event is stored in
|
||||||
or testing, without requiring the original sender to fire the webhook
|
full, which is what manual redelivery for debugging or testing will
|
||||||
again.
|
be built on. No redelivery exists today, in the web UI or the API;
|
||||||
|
see [TODO.md](TODO.md).
|
||||||
|
|
||||||
### Use Cases
|
### Use Cases
|
||||||
|
|
||||||
@@ -272,6 +278,7 @@ webhooker solves this by acting as a durable intermediary:
|
|||||||
size, and delivery performance
|
size, and delivery performance
|
||||||
- **Debugging** and introspection of webhook payloads in the web UI
|
- **Debugging** and introspection of webhook payloads in the web UI
|
||||||
- **Replay** of webhook events for application testing and development
|
- **Replay** of webhook events for application testing and development
|
||||||
|
(planned; not yet implemented)
|
||||||
- **Fan-out** delivery of a single webhook to multiple downstream
|
- **Fan-out** delivery of a single webhook to multiple downstream
|
||||||
targets
|
targets
|
||||||
- **High-availability ingestion** for delivery to less reliable backend
|
- **High-availability ingestion** for delivery to less reliable backend
|
||||||
@@ -497,7 +504,7 @@ A programmatic access credential for API authentication.
|
|||||||
#### Event
|
#### Event
|
||||||
|
|
||||||
A captured incoming webhook request. Stores the complete HTTP request
|
A captured incoming webhook request. Stores the complete HTTP request
|
||||||
data for replay and auditing.
|
data for auditing and for the planned replay capability.
|
||||||
|
|
||||||
| Field | Type | Description |
|
| Field | Type | Description |
|
||||||
| -------------- | ------ | ----------- |
|
| -------------- | ------ | ----------- |
|
||||||
@@ -779,9 +786,10 @@ unknown) one while one of its deliveries is still `retrying`, both
|
|||||||
recovery paths above terminally mark that delivery `failed` and record a
|
recovery paths above terminally mark that delivery `failed` and record a
|
||||||
`DeliveryResult` naming the current target type as the reason, logging it
|
`DeliveryResult` naming the current target type as the reason, logging it
|
||||||
at warn level. The delivery is not re-dispatched under the new type — the
|
at warn level. The delivery is not re-dispatched under the new type — the
|
||||||
operator never asked for that delivery — and the event itself remains
|
operator never asked for that delivery — and while the event itself
|
||||||
stored in the per-webhook event database, so it can be redelivered
|
remains stored in the per-webhook event database, there is no way to
|
||||||
manually.
|
redeliver it: manual redelivery is planned, not implemented (see
|
||||||
|
[TODO.md](TODO.md)).
|
||||||
|
|
||||||
### Circuit Breaker (HTTP Targets with Retries)
|
### Circuit Breaker (HTTP Targets with Retries)
|
||||||
|
|
||||||
@@ -1104,6 +1112,34 @@ binary is statically linked and runs on Alpine.
|
|||||||
`docker build .` is the CI gate — if it passes, the code is formatted,
|
`docker build .` is the CI gate — if it passes, the code is formatted,
|
||||||
linted, tested, and compiled.
|
linted, tested, and compiled.
|
||||||
|
|
||||||
|
#### CI gate honesty
|
||||||
|
|
||||||
|
A layer cache lets `docker build .` exit 0 in seconds with the lint and
|
||||||
|
test stages replayed rather than executed, which would make a green
|
||||||
|
check meaningless. The `check` workflow therefore writes
|
||||||
|
`.ci-fingerprint` into the build context before building. Its value is
|
||||||
|
the hash of the last commit that touched the build context, so:
|
||||||
|
|
||||||
|
- Any commit that changes code (including a squash merge whose tree
|
||||||
|
matches an already-built branch) gets a new fingerprint, invalidates
|
||||||
|
the `COPY . .` layer of both check stages, and really runs
|
||||||
|
`make fmt-check`, `make lint`, `make test`, and `make build`. A run
|
||||||
|
that reports success ran them.
|
||||||
|
- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
|
||||||
|
excludes `*.md` and `LICENSE` from the context anyway — so the image
|
||||||
|
replays from cache and costs seconds.
|
||||||
|
|
||||||
|
The module download layer sits above `COPY . .` and stays cached either
|
||||||
|
way.
|
||||||
|
|
||||||
|
The workflow's first step covers a second way the gate lied: Gitea
|
||||||
|
cancels an in-flight run when a newer commit lands on the same branch
|
||||||
|
and records that cancellation as a `failure` status, marking a commit
|
||||||
|
red that was never tested. Cancellation is unconditional server-side for
|
||||||
|
push events, so the superseding run rewrites the exact
|
||||||
|
`Has been cancelled` status to `skipped`. Genuine failures are never
|
||||||
|
touched.
|
||||||
|
|
||||||
## TODO
|
## TODO
|
||||||
|
|
||||||
See [TODO.md](TODO.md).
|
See [TODO.md](TODO.md).
|
||||||
|
|||||||
90
TODO.md
90
TODO.md
@@ -1,35 +1,94 @@
|
|||||||
# Workflow
|
# Workflow
|
||||||
|
|
||||||
* branch (from `main`)
|
One issue per unit of work, one branch and one PR per issue:
|
||||||
* do the work in Next Step
|
|
||||||
* move Next Step to the top of Completed Steps
|
* ensure a tracked issue exists with a definition of done
|
||||||
* move the top item of Future Steps into Next Step
|
* branch from `next` (never from `main`)
|
||||||
* commit (`TODO.md` changes in the same commit as the work)
|
* do the work; open a PR based on `next` (never on `main`)
|
||||||
* merge to `main` if the branch is not protected, otherwise open a PR
|
* pass an independent review, then the manager squash-merges into `next`
|
||||||
* push
|
* push; nothing stays local-only
|
||||||
|
|
||||||
|
`next` is the branch for the next milestone and must stay green and
|
||||||
|
mergeable to `main` without notice. One `next` -> `main` PR accumulates
|
||||||
|
the milestone; releases are cut from `main` separately.
|
||||||
|
|
||||||
|
Issue branches do NOT touch this file — the manager maintains it on
|
||||||
|
`next`. Every branch editing `TODO.md` conflicts with every other
|
||||||
|
(#112).
|
||||||
|
|
||||||
# Status
|
# Status
|
||||||
|
|
||||||
pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
|
pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
|
||||||
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
|
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
|
||||||
event retention (#63), the database archiving target (#43), the admin
|
event retention (#63), the database archiving target (#43), the admin
|
||||||
password change flow (#65), policy compliance (#6), pinned lint tooling
|
password change flow (#65), policy compliance (#6), pinned lint tooling
|
||||||
(#55), and fail-loud configuration parsing (#80). Note: TODO.md was
|
(#55), and fail-loud configuration parsing (#80).
|
||||||
deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
|
|
||||||
content was folded into the README TODO section, which this draft
|
`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
|
||||||
reconstructs as of 2026-07-06.
|
is closed, and it is verified green by cache-defeated container runs
|
||||||
|
rather than by the CI badge, which can pass without executing anything
|
||||||
|
(#119). Note: TODO.md was deliberately deleted from this repo in f9a9569
|
||||||
|
(2026-03-01, #6); its content was folded into the README TODO section,
|
||||||
|
which this draft reconstructs as of 2026-07-06.
|
||||||
|
|
||||||
# Next Step
|
# Next Step
|
||||||
|
|
||||||
Manual event redelivery from the web UI (replay is a core promised
|
Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
|
||||||
capability in the README rationale).
|
gate (#119) before the next cycle's work lands — a gate that can report
|
||||||
|
success without running is the one thing every other guarantee here
|
||||||
|
rests on.
|
||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
|
||||||
|
cap: the reverse walk cuts entries with `strings.LastIndexByte`
|
||||||
|
instead of joining and splitting, so a 1 MB header allocates 16 bytes
|
||||||
|
rather than 1.6 MB per request on the unauthenticated receiver.
|
||||||
|
Semantics proven unchanged by differential testing against the
|
||||||
|
previous implementation (#133)
|
||||||
|
- 2026-08-12 Cap the `X-Forwarded-For` hop walk at 64 entries, so an
|
||||||
|
attacker-supplied chain cannot burn unbounded CPU in the rate-limit
|
||||||
|
key function; running off the end falls back to the peer address
|
||||||
|
(#124)
|
||||||
|
- 2026-08-12 Gate forwarded-header trust behind a `TRUSTED_PROXIES` CIDR
|
||||||
|
list: all three rate limiters key on the connection's own address
|
||||||
|
unless the direct peer is a configured proxy, in which case
|
||||||
|
`X-Forwarded-For` is walked right to left for the first non-proxy hop.
|
||||||
|
Default trusts nothing, and a set-but-unparseable value aborts
|
||||||
|
startup. Before this, any client could mint a fresh bucket or drain
|
||||||
|
another's by rotating a spoofed header (#88)
|
||||||
- 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
|
- 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
|
||||||
Profile settings placeholder removed, a progressive-enhancement copy
|
Profile settings placeholder removed, a progressive-enhancement copy
|
||||||
button for the entrypoint URL, and retention form copy that states the
|
button for the entrypoint URL, and retention form copy that states the
|
||||||
actual policy (deletion by the reaper, 0 retains forever) (#57)
|
actual policy (deletion by the reaper, 0 retains forever) (#57)
|
||||||
|
- 2026-08-11 Mask the webhook credential in delivery errors and logs:
|
||||||
|
Go embeds the request URL in `*url.Error`, so every transport failure
|
||||||
|
persisted the full Slack webhook URL into the per-webhook event
|
||||||
|
database via `DeliveryResult.Error`, a field a future REST API would
|
||||||
|
have served. `maskURLError` drops path, query and userinfo while
|
||||||
|
preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
|
||||||
|
still work and DNS, TLS and timeout failures still read differently
|
||||||
|
(#118)
|
||||||
|
- 2026-08-11 Rate-limit the public webhook receiver endpoint
|
||||||
|
(`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
|
||||||
|
entrypoint path so one entrypoint cannot exhaust another's budget;
|
||||||
|
over-limit requests get 429 with `Retry-After`. It was the one
|
||||||
|
unauthenticated, internet-facing endpoint with no limit at all (#64)
|
||||||
|
- 2026-08-11 Enforce the body size limit before CSRF parses the form:
|
||||||
|
`MaxBodySize` is now first in all four form-parsing route groups, so
|
||||||
|
an oversized request is rejected with 413 instead of being read in
|
||||||
|
full by the CSRF middleware before any cap applied (#90)
|
||||||
|
- 2026-08-11 Mask target config on the source detail page, which
|
||||||
|
rendered the stored blob verbatim and so exposed the Slack
|
||||||
|
incoming-webhook URL — a bearer credential that cannot be revoked
|
||||||
|
per-holder. Config reaches the template only as a `TargetView` of
|
||||||
|
labelled fields, and header values are rendered as a count (#113)
|
||||||
|
- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
|
||||||
|
sentinel written in `BeforeSave` so the GORM column default cannot
|
||||||
|
win the race. Also bounds the reaper's cutoff arithmetic: day counts
|
||||||
|
above 106751 overflowed `time.Duration` and wrapped the cutoff into
|
||||||
|
the future, where every row matched and the sweep deleted everything
|
||||||
|
(#79)
|
||||||
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry
|
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry
|
||||||
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
|
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
|
||||||
requests, with the 7-day absolute cap kept as an independent
|
requests, with the 7-day absolute cap kept as an independent
|
||||||
@@ -84,6 +143,9 @@ capability in the README rationale).
|
|||||||
|
|
||||||
# Future Steps
|
# Future Steps
|
||||||
|
|
||||||
|
- Manual event redelivery from the web UI — the "Replay" capability the
|
||||||
|
README describes as planned. No redelivery code exists anywhere in the
|
||||||
|
tree; events are stored in full, which is all it would be built on
|
||||||
- Delivery status and retry management UI
|
- Delivery status and retry management UI
|
||||||
- Per-webhook rate limiting in the receiver handler (per-webhook config
|
- Per-webhook rate limiting in the receiver handler (per-webhook config
|
||||||
plus handler enforcement; global limits must not apply to receiver
|
plus handler enforcement; global limits must not apply to receiver
|
||||||
|
|||||||
@@ -92,6 +92,7 @@ type Config struct {
|
|||||||
SentryDSN string
|
SentryDSN string
|
||||||
|
|
||||||
// RetentionSweepInterval is how often the retention reaper runs.
|
// RetentionSweepInterval is how often the retention reaper runs.
|
||||||
|
// Always positive: it becomes a time.NewTicker period.
|
||||||
RetentionSweepInterval time.Duration
|
RetentionSweepInterval time.Duration
|
||||||
|
|
||||||
// SessionIdleTimeout is the sliding inactivity window after
|
// SessionIdleTimeout is the sliding inactivity window after
|
||||||
@@ -235,6 +236,34 @@ func envDuration(
|
|||||||
return d, nil
|
return d, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// envPositiveDuration returns the value of the named environment
|
||||||
|
// variable parsed as a Go duration that must be greater than zero.
|
||||||
|
// Returns defaultValue if not set. A set value that is unparseable or
|
||||||
|
// non-positive is a hard error naming the key and the bad value.
|
||||||
|
//
|
||||||
|
// This is for durations that reach time.NewTicker, which panics on a
|
||||||
|
// non-positive period, in a goroutine started after startup has
|
||||||
|
// already reported success. It is deliberately not used for durations
|
||||||
|
// where non-positive means "disabled" (SESSION_IDLE_TIMEOUT).
|
||||||
|
func envPositiveDuration(
|
||||||
|
key string,
|
||||||
|
defaultValue time.Duration,
|
||||||
|
) (time.Duration, error) {
|
||||||
|
d, err := envDuration(key, defaultValue)
|
||||||
|
if err != nil {
|
||||||
|
return 0, err
|
||||||
|
}
|
||||||
|
|
||||||
|
if d <= 0 {
|
||||||
|
return 0, fmt.Errorf(
|
||||||
|
"%w: %s must be greater than zero, got %s",
|
||||||
|
ErrNonPositiveValue, key, d,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return d, nil
|
||||||
|
}
|
||||||
|
|
||||||
// parseCIDR parses one trusted-proxy list entry, which may be a
|
// parseCIDR parses one trusted-proxy list entry, which may be a
|
||||||
// CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated
|
// CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated
|
||||||
// as a single-host block).
|
// as a single-host block).
|
||||||
@@ -346,7 +375,7 @@ func loadFromEnv() (*Config, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
retentionSweepInterval, err := envDuration(
|
retentionSweepInterval, err := envPositiveDuration(
|
||||||
"RETENTION_SWEEP_INTERVAL",
|
"RETENTION_SWEEP_INTERVAL",
|
||||||
defaultRetentionSweepInterval,
|
defaultRetentionSweepInterval,
|
||||||
)
|
)
|
||||||
@@ -354,6 +383,8 @@ func loadFromEnv() (*Config, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Non-positive is "disabled" here, not invalid, so this stays on
|
||||||
|
// envDuration.
|
||||||
sessionIdleTimeout, err := envDuration(
|
sessionIdleTimeout, err := envDuration(
|
||||||
"SESSION_IDLE_TIMEOUT",
|
"SESSION_IDLE_TIMEOUT",
|
||||||
defaultSessionIdleTimeout,
|
defaultSessionIdleTimeout,
|
||||||
|
|||||||
@@ -139,6 +139,10 @@ func TestRetentionSweepInterval(t *testing.T) {
|
|||||||
set bool
|
set bool
|
||||||
value string
|
value string
|
||||||
expectError bool
|
expectError bool
|
||||||
|
// sentinel, when set, must be wrapped by the startup
|
||||||
|
// error; every error case must additionally name the
|
||||||
|
// variable in its message.
|
||||||
|
sentinel error
|
||||||
expected time.Duration
|
expected time.Duration
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
@@ -158,6 +162,24 @@ func TestRetentionSweepInterval(t *testing.T) {
|
|||||||
value: "not-a-duration",
|
value: "not-a-duration",
|
||||||
expectError: true,
|
expectError: true,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
// A non-positive period panics the ticker in the
|
||||||
|
// reaper and archive-sweeper goroutines, long after
|
||||||
|
// startup has reported success, so it has to fail
|
||||||
|
// here instead.
|
||||||
|
name: "zero fails startup",
|
||||||
|
set: true,
|
||||||
|
value: "0s",
|
||||||
|
expectError: true,
|
||||||
|
sentinel: config.ErrNonPositiveValue,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "negative fails startup",
|
||||||
|
set: true,
|
||||||
|
value: "-1h",
|
||||||
|
expectError: true,
|
||||||
|
sentinel: config.ErrNonPositiveValue,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
@@ -175,7 +197,9 @@ func TestRetentionSweepInterval(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if tt.expectError {
|
if tt.expectError {
|
||||||
expectStartupError(t)
|
expectStartupErrorFor(
|
||||||
|
t, "RETENTION_SWEEP_INTERVAL", tt.sentinel,
|
||||||
|
)
|
||||||
} else {
|
} else {
|
||||||
testRetentionSweepIntervalSuccess(t, tt.expected)
|
testRetentionSweepIntervalSuccess(t, tt.expected)
|
||||||
}
|
}
|
||||||
@@ -281,6 +305,22 @@ func TestSessionIdleTimeout(t *testing.T) {
|
|||||||
value: "not-a-duration",
|
value: "not-a-duration",
|
||||||
expectError: true,
|
expectError: true,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
// Non-positive is "idle expiry disabled" for this
|
||||||
|
// variable, not a configuration error: unlike
|
||||||
|
// RETENTION_SWEEP_INTERVAL it never becomes a ticker
|
||||||
|
// period.
|
||||||
|
name: "zero disables idle expiry",
|
||||||
|
set: true,
|
||||||
|
value: "0s",
|
||||||
|
expected: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "negative disables idle expiry",
|
||||||
|
set: true,
|
||||||
|
value: "-1h",
|
||||||
|
expected: -time.Hour,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|||||||
@@ -46,19 +46,8 @@ func (r *RetentionReaper) ExportStart() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ExportStop stops the reaper's background loop for tests.
|
// ExportStop stops the reaper's background loop for tests.
|
||||||
func (r *RetentionReaper) ExportStop(ctx context.Context) error {
|
func (r *RetentionReaper) ExportStop() {
|
||||||
return r.stop(ctx)
|
r.stop()
|
||||||
}
|
|
||||||
|
|
||||||
// ExportWedgeLoop adds a goroutine to the reaper's WaitGroup that
|
|
||||||
// never observes cancellation and returns only when release is
|
|
||||||
// closed. It stands in for a sweep stuck on a locked database.
|
|
||||||
func (r *RetentionReaper) ExportWedgeLoop(
|
|
||||||
release <-chan struct{},
|
|
||||||
) {
|
|
||||||
r.wg.Go(func() {
|
|
||||||
<-release
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExportSetInterval overrides the sweep interval for tests.
|
// ExportSetInterval overrides the sweep interval for tests.
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import (
|
|||||||
"go.uber.org/fx"
|
"go.uber.org/fx"
|
||||||
"gorm.io/gorm"
|
"gorm.io/gorm"
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
|
||||||
"sneak.berlin/go/webhooker/internal/logger"
|
"sneak.berlin/go/webhooker/internal/logger"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -63,9 +62,8 @@ func NewRetentionReaper(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// registerHooks wires the reaper's start and stop into the fx
|
// registerHooks wires the reaper's start and stop into the fx
|
||||||
// lifecycle. The start hook's context is deliberately ignored (see
|
// lifecycle. The start hook's context is deliberately ignored: see
|
||||||
// start for why the sweep loop must not inherit it); the stop hook's
|
// start for why the sweep loop must not inherit it.
|
||||||
// context is honoured (see stop).
|
|
||||||
func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
|
func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
|
||||||
lc.Append(fx.Hook{
|
lc.Append(fx.Hook{
|
||||||
//nolint:contextcheck // Not inheriting the hook context is
|
//nolint:contextcheck // Not inheriting the hook context is
|
||||||
@@ -75,8 +73,10 @@ func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
OnStop: func(ctx context.Context) error {
|
OnStop: func(_ context.Context) error {
|
||||||
return r.stop(ctx)
|
r.stop()
|
||||||
|
|
||||||
|
return nil
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -105,27 +105,15 @@ func (r *RetentionReaper) start() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// stop cancels the sweep loop's context and waits for it to
|
func (r *RetentionReaper) stop() {
|
||||||
// exit, bounded by the stop hook's context: a sweep wedged on a
|
|
||||||
// locked database must not hang the process past fx's stop
|
|
||||||
// timeout.
|
|
||||||
func (r *RetentionReaper) stop(ctx context.Context) error {
|
|
||||||
r.log.Info("retention reaper stopping")
|
r.log.Info("retention reaper stopping")
|
||||||
|
|
||||||
if r.cancel != nil {
|
if r.cancel != nil {
|
||||||
r.cancel()
|
r.cancel()
|
||||||
}
|
}
|
||||||
|
|
||||||
err := lifecycle.WaitForShutdown(
|
r.wg.Wait()
|
||||||
ctx, r.log, "retention reaper", &r.wg,
|
|
||||||
)
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
r.log.Info("retention reaper stopped")
|
r.log.Info("retention reaper stopped")
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *RetentionReaper) run(ctx context.Context) {
|
func (r *RetentionReaper) run(ctx context.Context) {
|
||||||
|
|||||||
@@ -26,13 +26,6 @@ const (
|
|||||||
// reaperTestRetentionDays is the retention policy the lifecycle
|
// reaperTestRetentionDays is the retention policy the lifecycle
|
||||||
// tests give their webhook.
|
// tests give their webhook.
|
||||||
reaperTestRetentionDays = 30
|
reaperTestRetentionDays = 30
|
||||||
|
|
||||||
// reaperWedgeStopTimeout is the stop timeout the wedged-shutdown
|
|
||||||
// test hands OnStop, standing in for fx's StopTimeout. The test
|
|
||||||
// asserts only that the hook returns at all, and allows it
|
|
||||||
// reaperStopTimeout — forty times this budget — to do so, so no
|
|
||||||
// assertion races the wall clock.
|
|
||||||
reaperWedgeStopTimeout = 250 * time.Millisecond
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
||||||
@@ -214,59 +207,3 @@ func TestRetentionReaper_StopHookStopsLoop(t *testing.T) {
|
|||||||
"a stopped reaper must not sweep anything",
|
"a stopped reaper must not sweep anything",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestRetentionReaper_StopHookHonoursStopTimeout is the
|
|
||||||
// regression test for a shutdown that could never complete. fx
|
|
||||||
// hands OnStop a context carrying the application's stop timeout;
|
|
||||||
// an OnStop that discards it and calls wg.Wait() bare hangs the
|
|
||||||
// process forever on a sweep blocked on a locked SQLite database
|
|
||||||
// — precisely when a bounded shutdown matters most.
|
|
||||||
//
|
|
||||||
// The wedged goroutine here never observes cancellation, so the
|
|
||||||
// hook can only return by honouring its context, and it must say
|
|
||||||
// so rather than reporting a clean stop.
|
|
||||||
func TestRetentionReaper_StopHookHonoursStopTimeout(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := setupRetentionTest(t)
|
|
||||||
|
|
||||||
env.reaper.ExportSetInterval(reaperTestInterval)
|
|
||||||
|
|
||||||
lc := startReaperViaHook(t, env.reaper)
|
|
||||||
|
|
||||||
release := make(chan struct{})
|
|
||||||
|
|
||||||
t.Cleanup(func() { close(release) })
|
|
||||||
|
|
||||||
env.reaper.ExportWedgeLoop(release)
|
|
||||||
|
|
||||||
stopCtx, cancel := context.WithTimeout(
|
|
||||||
context.Background(), reaperWedgeStopTimeout,
|
|
||||||
)
|
|
||||||
defer cancel()
|
|
||||||
|
|
||||||
var stopErr error
|
|
||||||
|
|
||||||
stopped := make(chan struct{})
|
|
||||||
|
|
||||||
go func() {
|
|
||||||
defer close(stopped)
|
|
||||||
|
|
||||||
stopErr = lc.hooks[0].OnStop(stopCtx)
|
|
||||||
}()
|
|
||||||
|
|
||||||
select {
|
|
||||||
case <-stopped:
|
|
||||||
case <-time.After(reaperStopTimeout):
|
|
||||||
t.Fatal(
|
|
||||||
"OnStop did not return: it discarded the stop " +
|
|
||||||
"context and is waiting on a wedged goroutine " +
|
|
||||||
"that will never observe cancellation",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
require.ErrorIs(t, stopErr, context.DeadlineExceeded)
|
|
||||||
require.ErrorContains(t, stopErr, "retention reaper")
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import (
|
|||||||
"go.uber.org/fx"
|
"go.uber.org/fx"
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
|
||||||
"sneak.berlin/go/webhooker/internal/logger"
|
"sneak.berlin/go/webhooker/internal/logger"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -68,9 +67,10 @@ func NewArchiveSweeper(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// registerHooks wires the sweeper's start and stop into the fx
|
// registerHooks wires the sweeper's start and stop into the fx
|
||||||
// lifecycle. The start hook's context is deliberately ignored
|
// lifecycle. Both hook contexts are deliberately ignored: see
|
||||||
// (see start for why the background loop must not inherit it);
|
// start for why the background loop must not inherit the start
|
||||||
// the stop hook's context is honoured (see stop).
|
// hook's context, and stop for why shutdown blocks on the loop
|
||||||
|
// rather than on the stop hook's deadline.
|
||||||
func (s *ArchiveSweeper) registerHooks(lc fx.Lifecycle) {
|
func (s *ArchiveSweeper) registerHooks(lc fx.Lifecycle) {
|
||||||
lc.Append(fx.Hook{
|
lc.Append(fx.Hook{
|
||||||
//nolint:contextcheck // Not passing the hook context is
|
//nolint:contextcheck // Not passing the hook context is
|
||||||
@@ -80,8 +80,10 @@ func (s *ArchiveSweeper) registerHooks(lc fx.Lifecycle) {
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
OnStop: func(ctx context.Context) error {
|
OnStop: func(_ context.Context) error {
|
||||||
return s.stop(ctx)
|
s.stop()
|
||||||
|
|
||||||
|
return nil
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -111,27 +113,15 @@ func (s *ArchiveSweeper) start() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// stop cancels the sweep loop's context and waits for it to
|
func (s *ArchiveSweeper) stop() {
|
||||||
// exit, bounded by the stop hook's context: a prune wedged on a
|
|
||||||
// locked archive must not hang the process past fx's stop
|
|
||||||
// timeout.
|
|
||||||
func (s *ArchiveSweeper) stop(ctx context.Context) error {
|
|
||||||
s.log.Info("archive sweeper stopping")
|
s.log.Info("archive sweeper stopping")
|
||||||
|
|
||||||
if s.cancel != nil {
|
if s.cancel != nil {
|
||||||
s.cancel()
|
s.cancel()
|
||||||
}
|
}
|
||||||
|
|
||||||
err := lifecycle.WaitForShutdown(
|
s.wg.Wait()
|
||||||
ctx, s.log, "archive sweeper", &s.wg,
|
|
||||||
)
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
s.log.Info("archive sweeper stopped")
|
s.log.Info("archive sweeper stopped")
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *ArchiveSweeper) run(ctx context.Context) {
|
func (s *ArchiveSweeper) run(ctx context.Context) {
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import (
|
|||||||
"github.com/google/uuid"
|
"github.com/google/uuid"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
"go.uber.org/fx"
|
||||||
"gorm.io/driver/sqlite"
|
"gorm.io/driver/sqlite"
|
||||||
"gorm.io/gorm"
|
"gorm.io/gorm"
|
||||||
"gorm.io/gorm/clause"
|
"gorm.io/gorm/clause"
|
||||||
@@ -225,6 +226,17 @@ func countArchivedRows(path string) (int64, error) {
|
|||||||
return count, nil
|
return count, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// captureLifecycle is a minimal fx.Lifecycle that records the
|
||||||
|
// hooks a component registers, so a test can invoke the real
|
||||||
|
// OnStart/OnStop functions with a context of its choosing.
|
||||||
|
type captureLifecycle struct {
|
||||||
|
hooks []fx.Hook
|
||||||
|
}
|
||||||
|
|
||||||
|
func (l *captureLifecycle) Append(h fx.Hook) {
|
||||||
|
l.hooks = append(l.hooks, h)
|
||||||
|
}
|
||||||
|
|
||||||
// TestArchiveSweeper_LoopOutlivesStartHookContext is the
|
// TestArchiveSweeper_LoopOutlivesStartHookContext is the
|
||||||
// regression test for a sweeper that never swept. fx calls
|
// regression test for a sweeper that never swept. fx calls
|
||||||
// OnStart with a context carrying the application's start
|
// OnStart with a context carrying the application's start
|
||||||
@@ -258,7 +270,7 @@ func TestArchiveSweeper_LoopOutlivesStartHookContext(
|
|||||||
|
|
||||||
// Drive the genuine fx hooks the application registers,
|
// Drive the genuine fx hooks the application registers,
|
||||||
// rather than a test-only entry point.
|
// rather than a test-only entry point.
|
||||||
lc := &recordingLifecycle{}
|
lc := &captureLifecycle{}
|
||||||
env.sweeper.ExportRegisterHooks(lc)
|
env.sweeper.ExportRegisterHooks(lc)
|
||||||
require.Len(t, lc.hooks, 1)
|
require.Len(t, lc.hooks, 1)
|
||||||
|
|
||||||
@@ -912,36 +924,7 @@ func TestArchiveSweeper_StopsCleanly(t *testing.T) {
|
|||||||
env.sweeper.ExportSetInterval(time.Millisecond)
|
env.sweeper.ExportSetInterval(time.Millisecond)
|
||||||
env.sweeper.ExportStart()
|
env.sweeper.ExportStart()
|
||||||
|
|
||||||
// stop blocks on the loop's WaitGroup, so returning without
|
// stop blocks on the loop's WaitGroup, so returning at all
|
||||||
// error proves the loop observed the cancellation and exited
|
// proves the loop observed the cancellation and exited.
|
||||||
// well inside the stop context.
|
env.sweeper.ExportStop()
|
||||||
require.NoError(
|
|
||||||
t, env.sweeper.ExportStop(context.Background()),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestArchiveSweeper_StopHookHonoursStopTimeout is the sweeper's
|
|
||||||
// half of the same shutdown defect the engine and the retention
|
|
||||||
// reaper carried: an OnStop that discards its context and waits
|
|
||||||
// on the WaitGroup bare hangs the process forever on a prune
|
|
||||||
// wedged inside a locked archive.
|
|
||||||
func TestArchiveSweeper_StopHookHonoursStopTimeout(
|
|
||||||
t *testing.T,
|
|
||||||
) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
env := setupSweeperTest(t)
|
|
||||||
|
|
||||||
lc := &recordingLifecycle{}
|
|
||||||
env.sweeper.ExportRegisterHooks(lc)
|
|
||||||
require.Len(t, lc.hooks, 1)
|
|
||||||
require.NoError(t, lc.hooks[0].OnStart(context.Background()))
|
|
||||||
|
|
||||||
release := make(chan struct{})
|
|
||||||
|
|
||||||
t.Cleanup(func() { close(release) })
|
|
||||||
|
|
||||||
env.sweeper.ExportWedgeLoop(release)
|
|
||||||
|
|
||||||
requireStopHookExpires(t, lc.hooks[0], "archive sweeper")
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,7 +13,6 @@ import (
|
|||||||
"go.uber.org/fx"
|
"go.uber.org/fx"
|
||||||
"gorm.io/gorm"
|
"gorm.io/gorm"
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
|
||||||
"sneak.berlin/go/webhooker/internal/logger"
|
"sneak.berlin/go/webhooker/internal/logger"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -235,9 +234,8 @@ func (e *Engine) ScheduleRetry(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// registerHooks wires the engine's start and stop into the fx
|
// registerHooks wires the engine's start and stop into the fx
|
||||||
// lifecycle. The start hook's context is deliberately ignored
|
// lifecycle. The start hook's context is deliberately ignored:
|
||||||
// (see start for why the worker pool must not inherit it); the
|
// see start for why the worker pool must not inherit it.
|
||||||
// stop hook's context is honoured (see stop).
|
|
||||||
func (e *Engine) registerHooks(lc fx.Lifecycle) {
|
func (e *Engine) registerHooks(lc fx.Lifecycle) {
|
||||||
lc.Append(fx.Hook{
|
lc.Append(fx.Hook{
|
||||||
//nolint:contextcheck // Not inheriting the hook context
|
//nolint:contextcheck // Not inheriting the hook context
|
||||||
@@ -247,8 +245,10 @@ func (e *Engine) registerHooks(lc fx.Lifecycle) {
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
OnStop: func(ctx context.Context) error {
|
OnStop: func(_ context.Context) error {
|
||||||
return e.stop(ctx)
|
e.stop()
|
||||||
|
|
||||||
|
return nil
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -289,26 +289,11 @@ func (e *Engine) start() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// stop cancels the worker pool's context and waits for the pool
|
func (e *Engine) stop() {
|
||||||
// to drain, bounded by the stop hook's context: a wedged worker
|
|
||||||
// must not hang the process past fx's stop timeout.
|
|
||||||
func (e *Engine) stop(ctx context.Context) error {
|
|
||||||
e.log.Info("delivery engine stopping")
|
e.log.Info("delivery engine stopping")
|
||||||
|
|
||||||
if e.cancel != nil {
|
|
||||||
e.cancel()
|
e.cancel()
|
||||||
}
|
e.wg.Wait()
|
||||||
|
|
||||||
err := lifecycle.WaitForShutdown(
|
|
||||||
ctx, e.log, "delivery engine", &e.wg,
|
|
||||||
)
|
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
e.log.Info("delivery engine stopped")
|
e.log.Info("delivery engine stopped")
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (e *Engine) worker(ctx context.Context) {
|
func (e *Engine) worker(ctx context.Context) {
|
||||||
@@ -800,9 +785,9 @@ func (e *Engine) sweepSingleRetry(
|
|||||||
// status retrying themselves. Re-dispatching under the new type
|
// status retrying themselves. Re-dispatching under the new type
|
||||||
// would be a delivery the operator never asked for, and leaving
|
// would be a delivery the operator never asked for, and leaving
|
||||||
// the row retrying strands it forever, so the delivery is
|
// the row retrying strands it forever, so the delivery is
|
||||||
// failed with a recorded reason and can be redelivered
|
// failed with a recorded reason. The event stays stored, but
|
||||||
// manually. Logged at warn, not error: this is operator-caused
|
// nothing redelivers it today. Logged at warn, not error: this
|
||||||
// state, not a system fault.
|
// is operator-caused state, not a system fault.
|
||||||
func (e *Engine) failUnretryableRetry(
|
func (e *Engine) failUnretryableRetry(
|
||||||
webhookDB *gorm.DB,
|
webhookDB *gorm.DB,
|
||||||
webhookID string,
|
webhookID string,
|
||||||
|
|||||||
@@ -501,7 +501,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
|
|||||||
|
|
||||||
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
||||||
|
|
||||||
require.NoError(t, s.Engine.ExportStop(context.Background()))
|
s.Engine.ExportStop()
|
||||||
}
|
}
|
||||||
|
|
||||||
// iWaitForDelivered polls until the delivery reaches the
|
// iWaitForDelivered polls until the delivery reaches the
|
||||||
@@ -567,7 +567,7 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
|
|||||||
|
|
||||||
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
iWaitForDelivered(t, s.WebhookDB, d.ID)
|
||||||
|
|
||||||
require.NoError(t, s.Engine.ExportStop(context.Background()))
|
s.Engine.ExportStop()
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- processDelivery: unknown target type ---
|
// --- processDelivery: unknown target type ---
|
||||||
|
|||||||
@@ -27,13 +27,6 @@ const (
|
|||||||
// and a ready deliveryCh are chosen between at random and a
|
// and a ready deliveryCh are chosen between at random and a
|
||||||
// doomed pool still delivers.
|
// doomed pool still delivers.
|
||||||
hookSettleDelay = 250 * time.Millisecond
|
hookSettleDelay = 250 * time.Millisecond
|
||||||
|
|
||||||
// wedgeStopTimeout is the stop timeout a wedged-shutdown test
|
|
||||||
// hands OnStop, standing in for fx's StopTimeout. The test
|
|
||||||
// asserts only that the hook returns at all, and allows it
|
|
||||||
// hookStopTimeout — forty times this budget — to do so, so no
|
|
||||||
// assertion here races the wall clock.
|
|
||||||
wedgeStopTimeout = 250 * time.Millisecond
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
// recordingLifecycle is a minimal fx.Lifecycle that records the
|
||||||
@@ -47,44 +40,6 @@ func (l *recordingLifecycle) Append(h fx.Hook) {
|
|||||||
l.hooks = append(l.hooks, h)
|
l.hooks = append(l.hooks, h)
|
||||||
}
|
}
|
||||||
|
|
||||||
// requireStopHookExpires drives hook.OnStop with a stop context
|
|
||||||
// that expires while a wedged goroutine is still running, and
|
|
||||||
// requires the hook to return the deadline error naming
|
|
||||||
// component instead of blocking on the WaitGroup forever.
|
|
||||||
func requireStopHookExpires(
|
|
||||||
t *testing.T, hook fx.Hook, component string,
|
|
||||||
) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
stopCtx, cancel := context.WithTimeout(
|
|
||||||
context.Background(), wedgeStopTimeout,
|
|
||||||
)
|
|
||||||
defer cancel()
|
|
||||||
|
|
||||||
var stopErr error
|
|
||||||
|
|
||||||
stopped := make(chan struct{})
|
|
||||||
|
|
||||||
go func() {
|
|
||||||
defer close(stopped)
|
|
||||||
|
|
||||||
stopErr = hook.OnStop(stopCtx)
|
|
||||||
}()
|
|
||||||
|
|
||||||
select {
|
|
||||||
case <-stopped:
|
|
||||||
case <-time.After(hookStopTimeout):
|
|
||||||
t.Fatal(
|
|
||||||
"OnStop did not return: it discarded the stop " +
|
|
||||||
"context and is waiting on a wedged goroutine " +
|
|
||||||
"that will never observe cancellation",
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
require.ErrorIs(t, stopErr, context.DeadlineExceeded)
|
|
||||||
require.ErrorContains(t, stopErr, component)
|
|
||||||
}
|
|
||||||
|
|
||||||
// startEngineViaHook drives the genuine fx hooks the application
|
// startEngineViaHook drives the genuine fx hooks the application
|
||||||
// registers for the engine, handing OnStart a context that is
|
// registers for the engine, handing OnStart a context that is
|
||||||
// already done, and returns only once a pool that inherited that
|
// already done, and returns only once a pool that inherited that
|
||||||
@@ -242,30 +197,3 @@ func TestEngine_StopHookStopsWorkers(t *testing.T) {
|
|||||||
"a stopped engine must not deliver anything",
|
"a stopped engine must not deliver anything",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestEngine_StopHookHonoursStopTimeout is the regression test
|
|
||||||
// for a shutdown that could never complete. fx hands OnStop a
|
|
||||||
// context carrying the application's stop timeout; an OnStop
|
|
||||||
// that discards it and calls wg.Wait() bare hangs the process
|
|
||||||
// forever on a single worker stuck inside a delivery target that
|
|
||||||
// never returns — precisely when a bounded shutdown matters
|
|
||||||
// most.
|
|
||||||
//
|
|
||||||
// The wedged goroutine here never observes cancellation, so the
|
|
||||||
// hook can only return by honouring its context, and it must say
|
|
||||||
// so rather than reporting a clean stop.
|
|
||||||
func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s := newISetup(t)
|
|
||||||
|
|
||||||
lc := startEngineViaHook(t, s.Engine)
|
|
||||||
|
|
||||||
release := make(chan struct{})
|
|
||||||
|
|
||||||
t.Cleanup(func() { close(release) })
|
|
||||||
|
|
||||||
s.Engine.ExportWedgeWorker(release)
|
|
||||||
|
|
||||||
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -216,19 +216,8 @@ func (e *Engine) ExportRegisterHooks(lc fx.Lifecycle) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ExportStop exposes stop for testing.
|
// ExportStop exposes stop for testing.
|
||||||
func (e *Engine) ExportStop(ctx context.Context) error {
|
func (e *Engine) ExportStop() {
|
||||||
return e.stop(ctx)
|
e.stop()
|
||||||
}
|
|
||||||
|
|
||||||
// ExportWedgeWorker adds a goroutine to the engine's WaitGroup
|
|
||||||
// that never observes cancellation and returns only when release
|
|
||||||
// is closed. It stands in for a worker stuck inside a delivery
|
|
||||||
// target that never returns, which is the only way stop can be
|
|
||||||
// made to outlast its context.
|
|
||||||
func (e *Engine) ExportWedgeWorker(release <-chan struct{}) {
|
|
||||||
e.wg.Go(func() {
|
|
||||||
<-release
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExportDeliveryCh returns the delivery channel.
|
// ExportDeliveryCh returns the delivery channel.
|
||||||
@@ -529,19 +518,8 @@ func (s *ArchiveSweeper) ExportRegisterHooks(lc fx.Lifecycle) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ExportStop stops the sweeper's background loop for tests.
|
// ExportStop stops the sweeper's background loop for tests.
|
||||||
func (s *ArchiveSweeper) ExportStop(ctx context.Context) error {
|
func (s *ArchiveSweeper) ExportStop() {
|
||||||
return s.stop(ctx)
|
s.stop()
|
||||||
}
|
|
||||||
|
|
||||||
// ExportWedgeLoop adds a goroutine to the sweeper's WaitGroup
|
|
||||||
// that never observes cancellation and returns only when release
|
|
||||||
// is closed. It stands in for a prune stuck on a locked archive.
|
|
||||||
func (s *ArchiveSweeper) ExportWedgeLoop(
|
|
||||||
release <-chan struct{},
|
|
||||||
) {
|
|
||||||
s.wg.Go(func() {
|
|
||||||
<-release
|
|
||||||
})
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ExportSetInterval overrides the sweep interval for tests.
|
// ExportSetInterval overrides the sweep interval for tests.
|
||||||
|
|||||||
@@ -1,57 +0,0 @@
|
|||||||
// Package lifecycle holds helpers shared by the components that
|
|
||||||
// register fx start and stop hooks.
|
|
||||||
package lifecycle
|
|
||||||
|
|
||||||
import (
|
|
||||||
"context"
|
|
||||||
"fmt"
|
|
||||||
"log/slog"
|
|
||||||
"sync"
|
|
||||||
)
|
|
||||||
|
|
||||||
// WaitForShutdown waits for wg to drain, bounded by ctx.
|
|
||||||
//
|
|
||||||
// fx hands OnStop a context carrying the application's stop
|
|
||||||
// timeout. A bare wg.Wait() discards that deadline, so a single
|
|
||||||
// goroutine that never observes cancellation — a delivery target
|
|
||||||
// that never returns, a SQLite operation blocked on a lock —
|
|
||||||
// hangs the process forever instead of letting it exit when the
|
|
||||||
// timeout expires, which is exactly when a clean shutdown matters
|
|
||||||
// most.
|
|
||||||
//
|
|
||||||
// On timeout it logs at error naming component and returns an
|
|
||||||
// error: the goroutines are still running, and reporting success
|
|
||||||
// would hide an unclean shutdown from the operator. The waiting
|
|
||||||
// goroutine outlives this call and exits when (if) wg drains; it
|
|
||||||
// holds nothing but the channel it closes.
|
|
||||||
func WaitForShutdown(
|
|
||||||
ctx context.Context,
|
|
||||||
log *slog.Logger,
|
|
||||||
component string,
|
|
||||||
wg *sync.WaitGroup,
|
|
||||||
) error {
|
|
||||||
done := make(chan struct{})
|
|
||||||
|
|
||||||
go func() {
|
|
||||||
defer close(done)
|
|
||||||
|
|
||||||
wg.Wait()
|
|
||||||
}()
|
|
||||||
|
|
||||||
select {
|
|
||||||
case <-done:
|
|
||||||
return nil
|
|
||||||
case <-ctx.Done():
|
|
||||||
log.Error(
|
|
||||||
"shutdown timed out, goroutines still running",
|
|
||||||
"component", component,
|
|
||||||
"error", ctx.Err(),
|
|
||||||
)
|
|
||||||
|
|
||||||
return fmt.Errorf(
|
|
||||||
"%s: shutdown timed out, "+
|
|
||||||
"goroutines still running: %w",
|
|
||||||
component, ctx.Err(),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,62 +0,0 @@
|
|||||||
package lifecycle_test
|
|
||||||
|
|
||||||
import (
|
|
||||||
"context"
|
|
||||||
"log/slog"
|
|
||||||
"sync"
|
|
||||||
"testing"
|
|
||||||
"time"
|
|
||||||
|
|
||||||
"github.com/stretchr/testify/require"
|
|
||||||
"sneak.berlin/go/webhooker/internal/lifecycle"
|
|
||||||
)
|
|
||||||
|
|
||||||
// waitTimeout is the stop budget the timeout case gives a
|
|
||||||
// goroutine that never returns. The test's own patience is the
|
|
||||||
// go test deadline, so the only thing this value affects is how
|
|
||||||
// long the case takes.
|
|
||||||
const waitTimeout = 100 * time.Millisecond
|
|
||||||
|
|
||||||
func discardLogger() *slog.Logger {
|
|
||||||
return slog.New(slog.DiscardHandler)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestWaitForShutdown_DrainedGroup(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
var wg sync.WaitGroup
|
|
||||||
|
|
||||||
wg.Go(func() {})
|
|
||||||
|
|
||||||
require.NoError(
|
|
||||||
t,
|
|
||||||
lifecycle.WaitForShutdown(
|
|
||||||
context.Background(), discardLogger(),
|
|
||||||
"test component", &wg,
|
|
||||||
),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestWaitForShutdown_ContextExpires(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
release := make(chan struct{})
|
|
||||||
|
|
||||||
t.Cleanup(func() { close(release) })
|
|
||||||
|
|
||||||
var wg sync.WaitGroup
|
|
||||||
|
|
||||||
wg.Go(func() { <-release })
|
|
||||||
|
|
||||||
ctx, cancel := context.WithTimeout(
|
|
||||||
context.Background(), waitTimeout,
|
|
||||||
)
|
|
||||||
defer cancel()
|
|
||||||
|
|
||||||
err := lifecycle.WaitForShutdown(
|
|
||||||
ctx, discardLogger(), "test component", &wg,
|
|
||||||
)
|
|
||||||
|
|
||||||
require.ErrorIs(t, err, context.DeadlineExceeded)
|
|
||||||
require.ErrorContains(t, err, "test component")
|
|
||||||
}
|
|
||||||
@@ -25,6 +25,11 @@ func IPFromHostPort(hp string) string {
|
|||||||
return ipFromHostPort(hp)
|
return ipFromHostPort(hp)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ClientKeyForTest exposes clientKey for testing.
|
||||||
|
func ClientKeyForTest(m *Middleware, r *http.Request) string {
|
||||||
|
return m.clientKey(r)
|
||||||
|
}
|
||||||
|
|
||||||
// IsClientTLS exposes isClientTLS for testing.
|
// IsClientTLS exposes isClientTLS for testing.
|
||||||
func IsClientTLS(r *http.Request) bool {
|
func IsClientTLS(r *http.Request) bool {
|
||||||
return isClientTLS(r)
|
return isClientTLS(r)
|
||||||
|
|||||||
@@ -32,6 +32,13 @@ const (
|
|||||||
// receiver rate limit. The configured limit is expressed in
|
// receiver rate limit. The configured limit is expressed in
|
||||||
// requests per minute.
|
// requests per minute.
|
||||||
receiverRateInterval = 1 * time.Minute
|
receiverRateInterval = 1 * time.Minute
|
||||||
|
|
||||||
|
// maxForwardedHops bounds how many X-Forwarded-For entries the
|
||||||
|
// chain walk examines. Real chains are one to three hops, but a
|
||||||
|
// client can pad the header up to MaxHeaderBytes, so without a
|
||||||
|
// bound every request pays a walk proportional to whatever the
|
||||||
|
// client sent.
|
||||||
|
maxForwardedHops = 64
|
||||||
)
|
)
|
||||||
|
|
||||||
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
|
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
|
||||||
@@ -70,14 +77,36 @@ func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
|
|||||||
// a trusted proxy is the client. A hop that cannot be read as a bare
|
// a trusted proxy is the client. A hop that cannot be read as a bare
|
||||||
// address ends the walk: past it the chain is not the shape assumed
|
// address ends the walk: past it the chain is not the shape assumed
|
||||||
// here, so the caller falls back to the peer address.
|
// here, so the caller falls back to the peer address.
|
||||||
|
//
|
||||||
|
// Only the last maxForwardedHops entries are examined. A longer chain
|
||||||
|
// is padding, and running out of hops falls back to the peer address
|
||||||
|
// the same way an unreadable hop does.
|
||||||
|
//
|
||||||
|
// The entries are cut off the right end of each header value in place
|
||||||
|
// rather than split out of it: the receiver is unauthenticated and a
|
||||||
|
// client can pad the header up to MaxHeaderBytes, so splitting would
|
||||||
|
// allocate in proportion to the padding (about 8 MB for a 1 MB
|
||||||
|
// header) before the cap could discard any of it. Multiple header
|
||||||
|
// values are walked in reverse for the same reason, since joining
|
||||||
|
// them copies the whole chain.
|
||||||
func (m *Middleware) forwardedClientAddr(
|
func (m *Middleware) forwardedClientAddr(
|
||||||
r *http.Request,
|
r *http.Request,
|
||||||
) (netip.Addr, bool) {
|
) (netip.Addr, bool) {
|
||||||
hops := strings.Split(
|
seen := 0
|
||||||
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
|
|
||||||
)
|
for _, value := range slices.Backward(
|
||||||
|
r.Header.Values("X-Forwarded-For"),
|
||||||
|
) {
|
||||||
|
for last := false; !last && seen < maxForwardedHops; seen++ {
|
||||||
|
hop := value
|
||||||
|
|
||||||
|
comma := strings.LastIndexByte(value, ',')
|
||||||
|
if comma < 0 {
|
||||||
|
last = true
|
||||||
|
} else {
|
||||||
|
hop, value = value[comma+1:], value[:comma]
|
||||||
|
}
|
||||||
|
|
||||||
for _, hop := range slices.Backward(hops) {
|
|
||||||
hop = strings.TrimSpace(hop)
|
hop = strings.TrimSpace(hop)
|
||||||
if hop == "" {
|
if hop == "" {
|
||||||
continue
|
continue
|
||||||
@@ -92,6 +121,7 @@ func (m *Middleware) forwardedClientAddr(
|
|||||||
return addr, true
|
return addr, true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return netip.Addr{}, false
|
return netip.Addr{}, false
|
||||||
}
|
}
|
||||||
@@ -113,8 +143,10 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
|||||||
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
|
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// Not an address we can reason about; key on the raw
|
// Not an address we can reason about; key on the raw
|
||||||
// value rather than collapsing such peers into one
|
// value, the most specific identity left. On a
|
||||||
// shared bucket.
|
// Unix-socket listener every peer carries the same
|
||||||
|
// RemoteAddr and so shares one bucket, which is the
|
||||||
|
// fail-closed direction.
|
||||||
return r.RemoteAddr
|
return r.RemoteAddr
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -8,7 +8,10 @@ import (
|
|||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"net/netip"
|
"net/netip"
|
||||||
"os"
|
"os"
|
||||||
|
"runtime"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
@@ -568,6 +571,105 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer covers the
|
||||||
|
// hop-walk cap. A client behind the trusted proxy can pad
|
||||||
|
// X-Forwarded-For with tens of thousands of trusted-looking hops,
|
||||||
|
// which costs a walk proportional to the padding and, once the walk
|
||||||
|
// runs off the left end of the chain, reaches the entry the client
|
||||||
|
// put there. Capping the walk stops both: the key falls back to the
|
||||||
|
// peer address, so rotating the head of the chain mints no bucket,
|
||||||
|
// and the run does not scale with the chain length.
|
||||||
|
func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// 50k hops is roughly 0.9 MB, within the default
|
||||||
|
// MaxHeaderBytes.
|
||||||
|
const hops = 50000
|
||||||
|
|
||||||
|
padding := strings.Repeat(", 10.0.0.2", hops-1)
|
||||||
|
|
||||||
|
start := time.Now()
|
||||||
|
|
||||||
|
assertSharedBucket(
|
||||||
|
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||||
|
func(i int) map[string]string {
|
||||||
|
return map[string]string{
|
||||||
|
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"a padded X-Forwarded-For chain must fall back to the "+
|
||||||
|
"peer address, not reach the client-controlled entry "+
|
||||||
|
"at the head of the chain",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.Less(
|
||||||
|
t, time.Since(start), 2*time.Second,
|
||||||
|
"the capped walk must not scale with the chain length",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRateLimitKey_LongChainAllocationIsBounded is the allocation
|
||||||
|
// half of the hop cap. Capping the walk still left every request
|
||||||
|
// paying for the whole header the client sent, because the chain was
|
||||||
|
// split before it was capped: about 8 MB of []string for the 1 MB a
|
||||||
|
// default MaxHeaderBytes allows, on the unauthenticated receiver.
|
||||||
|
//
|
||||||
|
// Bytes are the measurement, not allocation count: strings.Split of a
|
||||||
|
// 1 MB chain is a single allocation, so testing.AllocsPerRun scores
|
||||||
|
// it as cheap. The test is deliberately sequential — it reads
|
||||||
|
// process-wide counters, and Go runs this package's parallel tests
|
||||||
|
// only after the sequential ones finish.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // reads process-wide allocation counters
|
||||||
|
func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
|
||||||
|
// 100k hops of ", 10.0.0.2" is roughly 1 MB.
|
||||||
|
const (
|
||||||
|
hops = 100000
|
||||||
|
iterations = 50
|
||||||
|
maxBytesPerCall = 4096
|
||||||
|
)
|
||||||
|
|
||||||
|
m := rateLimitMiddleware(t, &config.Config{
|
||||||
|
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||||
|
})
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(), http.MethodPost, loginPath, nil,
|
||||||
|
)
|
||||||
|
req.RemoteAddr = "10.0.0.1:44444"
|
||||||
|
req.Header.Set(
|
||||||
|
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
|
||||||
|
)
|
||||||
|
|
||||||
|
var before, after runtime.MemStats
|
||||||
|
|
||||||
|
var key string
|
||||||
|
|
||||||
|
runtime.ReadMemStats(&before)
|
||||||
|
|
||||||
|
for range iterations {
|
||||||
|
key = middleware.ClientKeyForTest(m, req)
|
||||||
|
}
|
||||||
|
|
||||||
|
runtime.ReadMemStats(&after)
|
||||||
|
|
||||||
|
perCall := (after.TotalAlloc - before.TotalAlloc) / iterations
|
||||||
|
|
||||||
|
assert.Less(
|
||||||
|
t, perCall, uint64(maxBytesPerCall),
|
||||||
|
"a %d-byte X-Forwarded-For must not allocate in proportion "+
|
||||||
|
"to its length, but cost %d bytes per call",
|
||||||
|
len(req.Header.Get(headerXFF)), perCall,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t, "10.0.0.1", key,
|
||||||
|
"the padded chain must still fall back to the peer address",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
|
// TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer proves
|
||||||
// the receiver limiter uses the same gated key function as the
|
// the receiver limiter uses the same gated key function as the
|
||||||
// POST limiters.
|
// POST limiters.
|
||||||
|
|||||||
@@ -1,5 +1,4 @@
|
|||||||
// Webhooker client-side JavaScript
|
// Webhooker client-side JavaScript
|
||||||
console.log("Webhooker loaded");
|
|
||||||
|
|
||||||
// Copy-to-clipboard, as progressive enhancement.
|
// Copy-to-clipboard, as progressive enhancement.
|
||||||
//
|
//
|
||||||
|
|||||||
Reference in New Issue
Block a user