Compare commits
1 Commits
c4022c0834
...
issue-102-
| Author | SHA1 | Date | |
|---|---|---|---|
| e83eb2977e |
@@ -1,6 +1,3 @@
|
|||||||
# .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,53 +11,5 @@ 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
|
||||||
|
|||||||
5
.gitignore
vendored
5
.gitignore
vendored
@@ -41,7 +41,4 @@ data/
|
|||||||
|
|
||||||
# Temporary files
|
# Temporary files
|
||||||
tmp/
|
tmp/
|
||||||
temp/
|
temp/
|
||||||
|
|
||||||
# CI cache barrier, written into the build context by the check workflow
|
|
||||||
.ci-fingerprint
|
|
||||||
@@ -12,11 +12,7 @@ WORKDIR /src
|
|||||||
COPY go.mod go.sum ./
|
COPY go.mod go.sum ./
|
||||||
RUN go mod download
|
RUN go mod download
|
||||||
|
|
||||||
# Copy source code. In CI the context also carries .ci-fingerprint, whose
|
# Copy source code
|
||||||
# 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
|
||||||
@@ -40,8 +36,7 @@ WORKDIR /build
|
|||||||
COPY go.mod go.sum ./
|
COPY go.mod go.sum ./
|
||||||
RUN go mod download
|
RUN go mod download
|
||||||
|
|
||||||
# Copy source code, including the .ci-fingerprint cache barrier described in
|
# Copy source code
|
||||||
# the lint stage above.
|
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
# Run tests and build
|
# Run tests and build
|
||||||
|
|||||||
54
README.md
54
README.md
@@ -93,7 +93,6 @@ 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) |
|
||||||
@@ -174,12 +173,8 @@ 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,
|
`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in
|
||||||
`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
|
`TRUSTED_PROXIES` must be a CIDR block or a bare IP address.
|
||||||
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`,
|
||||||
@@ -265,10 +260,9 @@ 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** (not yet implemented) — Every received event is stored in
|
5. **Replay** — Stored events can be manually redelivered for debugging
|
||||||
full, which is what manual redelivery for debugging or testing will
|
or testing, without requiring the original sender to fire the webhook
|
||||||
be built on. No redelivery exists today, in the web UI or the API;
|
again.
|
||||||
see [TODO.md](TODO.md).
|
|
||||||
|
|
||||||
### Use Cases
|
### Use Cases
|
||||||
|
|
||||||
@@ -278,7 +272,6 @@ 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
|
||||||
@@ -504,7 +497,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 auditing and for the planned replay capability.
|
data for replay and auditing.
|
||||||
|
|
||||||
| Field | Type | Description |
|
| Field | Type | Description |
|
||||||
| -------------- | ------ | ----------- |
|
| -------------- | ------ | ----------- |
|
||||||
@@ -786,10 +779,9 @@ 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 while the event itself
|
operator never asked for that delivery — and the event itself remains
|
||||||
remains stored in the per-webhook event database, there is no way to
|
stored in the per-webhook event database, so it can be redelivered
|
||||||
redeliver it: manual redelivery is planned, not implemented (see
|
manually.
|
||||||
[TODO.md](TODO.md)).
|
|
||||||
|
|
||||||
### Circuit Breaker (HTTP Targets with Retries)
|
### Circuit Breaker (HTTP Targets with Retries)
|
||||||
|
|
||||||
@@ -1112,34 +1104,6 @@ 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,94 +1,35 @@
|
|||||||
# Workflow
|
# Workflow
|
||||||
|
|
||||||
One issue per unit of work, one branch and one PR per issue:
|
* branch (from `main`)
|
||||||
|
* do the work in Next Step
|
||||||
* ensure a tracked issue exists with a definition of done
|
* move Next Step to the top of Completed Steps
|
||||||
* branch from `next` (never from `main`)
|
* move the top item of Future Steps into Next Step
|
||||||
* do the work; open a PR based on `next` (never on `main`)
|
* commit (`TODO.md` changes in the same commit as the work)
|
||||||
* pass an independent review, then the manager squash-merges into `next`
|
* merge to `main` if the branch is not protected, otherwise open a PR
|
||||||
* push; nothing stays local-only
|
* push
|
||||||
|
|
||||||
`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).
|
(#55), and fail-loud configuration parsing (#80). Note: TODO.md was
|
||||||
|
deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
|
||||||
`next` (9bfd033) holds the completed 1.0.0 milestone: every issue in it
|
content was folded into the README TODO section, which this draft
|
||||||
is closed, and it is verified green by cache-defeated container runs
|
reconstructs as of 2026-07-06.
|
||||||
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
|
||||||
|
|
||||||
Tag 1.0.0 from `main` once the milestone PR merges, then repair the CI
|
Manual event redelivery from the web UI (replay is a core promised
|
||||||
gate (#119) before the next cycle's work lands — a gate that can report
|
capability in the README rationale).
|
||||||
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
|
||||||
@@ -143,9 +84,6 @@ rests on.
|
|||||||
|
|
||||||
# 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,7 +92,6 @@ 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
|
||||||
@@ -236,34 +235,6 @@ 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).
|
||||||
@@ -375,7 +346,7 @@ func loadFromEnv() (*Config, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
retentionSweepInterval, err := envPositiveDuration(
|
retentionSweepInterval, err := envDuration(
|
||||||
"RETENTION_SWEEP_INTERVAL",
|
"RETENTION_SWEEP_INTERVAL",
|
||||||
defaultRetentionSweepInterval,
|
defaultRetentionSweepInterval,
|
||||||
)
|
)
|
||||||
@@ -383,8 +354,6 @@ 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,11 +139,7 @@ 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
|
expected time.Duration
|
||||||
// error; every error case must additionally name the
|
|
||||||
// variable in its message.
|
|
||||||
sentinel error
|
|
||||||
expected time.Duration
|
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: caseUnsetUsesDefault,
|
name: caseUnsetUsesDefault,
|
||||||
@@ -162,24 +158,6 @@ 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 {
|
||||||
@@ -197,9 +175,7 @@ func TestRetentionSweepInterval(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if tt.expectError {
|
if tt.expectError {
|
||||||
expectStartupErrorFor(
|
expectStartupError(t)
|
||||||
t, "RETENTION_SWEEP_INTERVAL", tt.sentinel,
|
|
||||||
)
|
|
||||||
} else {
|
} else {
|
||||||
testRetentionSweepIntervalSuccess(t, tt.expected)
|
testRetentionSweepIntervalSuccess(t, tt.expected)
|
||||||
}
|
}
|
||||||
@@ -305,22 +281,6 @@ 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,8 +46,19 @@ 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() {
|
func (r *RetentionReaper) ExportStop(ctx context.Context) error {
|
||||||
r.stop()
|
return r.stop(ctx)
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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,6 +10,7 @@ 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"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -62,8 +63,9 @@ 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.
|
// start for why the sweep loop must not inherit it); the stop hook's
|
||||||
|
// 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
|
||||||
@@ -73,10 +75,8 @@ func (r *RetentionReaper) registerHooks(lc fx.Lifecycle) {
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
OnStop: func(_ context.Context) error {
|
OnStop: func(ctx context.Context) error {
|
||||||
r.stop()
|
return r.stop(ctx)
|
||||||
|
|
||||||
return nil
|
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -105,15 +105,27 @@ func (r *RetentionReaper) start() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (r *RetentionReaper) stop() {
|
// stop cancels the sweep loop's context and waits for it to
|
||||||
|
// 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()
|
||||||
}
|
}
|
||||||
|
|
||||||
r.wg.Wait()
|
err := lifecycle.WaitForShutdown(
|
||||||
|
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,6 +26,13 @@ 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
|
||||||
@@ -207,3 +214,59 @@ 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,6 +10,7 @@ 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"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -67,10 +68,9 @@ 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. Both hook contexts are deliberately ignored: see
|
// lifecycle. The start hook's context is deliberately ignored
|
||||||
// start for why the background loop must not inherit the start
|
// (see start for why the background loop must not inherit it);
|
||||||
// hook's context, and stop for why shutdown blocks on the loop
|
// the stop hook's context is honoured (see stop).
|
||||||
// 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,10 +80,8 @@ func (s *ArchiveSweeper) registerHooks(lc fx.Lifecycle) {
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
OnStop: func(_ context.Context) error {
|
OnStop: func(ctx context.Context) error {
|
||||||
s.stop()
|
return s.stop(ctx)
|
||||||
|
|
||||||
return nil
|
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -113,15 +111,27 @@ func (s *ArchiveSweeper) start() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *ArchiveSweeper) stop() {
|
// stop cancels the sweep loop's context and waits for it to
|
||||||
|
// 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()
|
||||||
}
|
}
|
||||||
|
|
||||||
s.wg.Wait()
|
err := lifecycle.WaitForShutdown(
|
||||||
|
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,7 +14,6 @@ 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"
|
||||||
@@ -226,17 +225,6 @@ 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
|
||||||
@@ -270,7 +258,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 := &captureLifecycle{}
|
lc := &recordingLifecycle{}
|
||||||
env.sweeper.ExportRegisterHooks(lc)
|
env.sweeper.ExportRegisterHooks(lc)
|
||||||
require.Len(t, lc.hooks, 1)
|
require.Len(t, lc.hooks, 1)
|
||||||
|
|
||||||
@@ -924,7 +912,36 @@ 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 at all
|
// stop blocks on the loop's WaitGroup, so returning without
|
||||||
// proves the loop observed the cancellation and exited.
|
// error proves the loop observed the cancellation and exited
|
||||||
env.sweeper.ExportStop()
|
// well inside the stop context.
|
||||||
|
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,6 +13,7 @@ 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"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -234,8 +235,9 @@ 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.
|
// (see start for why the worker pool must not inherit it); the
|
||||||
|
// 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
|
||||||
@@ -245,10 +247,8 @@ func (e *Engine) registerHooks(lc fx.Lifecycle) {
|
|||||||
|
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
OnStop: func(_ context.Context) error {
|
OnStop: func(ctx context.Context) error {
|
||||||
e.stop()
|
return e.stop(ctx)
|
||||||
|
|
||||||
return nil
|
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -289,11 +289,26 @@ func (e *Engine) start() {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (e *Engine) stop() {
|
// stop cancels the worker pool's context and waits for the pool
|
||||||
|
// 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")
|
||||||
e.cancel()
|
|
||||||
e.wg.Wait()
|
if e.cancel != nil {
|
||||||
|
e.cancel()
|
||||||
|
}
|
||||||
|
|
||||||
|
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) {
|
||||||
@@ -785,9 +800,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. The event stays stored, but
|
// failed with a recorded reason and can be redelivered
|
||||||
// nothing redelivers it today. Logged at warn, not error: this
|
// manually. Logged at warn, not error: this is operator-caused
|
||||||
// is operator-caused state, not a system fault.
|
// 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)
|
||||||
|
|
||||||
s.Engine.ExportStop()
|
require.NoError(t, s.Engine.ExportStop(context.Background()))
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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)
|
||||||
|
|
||||||
s.Engine.ExportStop()
|
require.NoError(t, s.Engine.ExportStop(context.Background()))
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- processDelivery: unknown target type ---
|
// --- processDelivery: unknown target type ---
|
||||||
|
|||||||
@@ -27,6 +27,13 @@ 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
|
||||||
@@ -40,6 +47,44 @@ 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
|
||||||
@@ -197,3 +242,30 @@ 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,8 +216,19 @@ func (e *Engine) ExportRegisterHooks(lc fx.Lifecycle) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ExportStop exposes stop for testing.
|
// ExportStop exposes stop for testing.
|
||||||
func (e *Engine) ExportStop() {
|
func (e *Engine) ExportStop(ctx context.Context) error {
|
||||||
e.stop()
|
return e.stop(ctx)
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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.
|
||||||
@@ -518,8 +529,19 @@ 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() {
|
func (s *ArchiveSweeper) ExportStop(ctx context.Context) error {
|
||||||
s.stop()
|
return s.stop(ctx)
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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.
|
||||||
|
|||||||
57
internal/lifecycle/lifecycle.go
Normal file
57
internal/lifecycle/lifecycle.go
Normal file
@@ -0,0 +1,57 @@
|
|||||||
|
// 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(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
62
internal/lifecycle/lifecycle_test.go
Normal file
62
internal/lifecycle/lifecycle_test.go
Normal file
@@ -0,0 +1,62 @@
|
|||||||
|
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,11 +25,6 @@ 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,13 +32,6 @@ 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
|
||||||
@@ -77,49 +70,26 @@ 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) {
|
||||||
seen := 0
|
hops := strings.Split(
|
||||||
|
strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",",
|
||||||
|
)
|
||||||
|
|
||||||
for _, value := range slices.Backward(
|
for _, hop := range slices.Backward(hops) {
|
||||||
r.Header.Values("X-Forwarded-For"),
|
hop = strings.TrimSpace(hop)
|
||||||
) {
|
if hop == "" {
|
||||||
for last := false; !last && seen < maxForwardedHops; seen++ {
|
continue
|
||||||
hop := value
|
}
|
||||||
|
|
||||||
comma := strings.LastIndexByte(value, ',')
|
addr, err := netip.ParseAddr(hop)
|
||||||
if comma < 0 {
|
if err != nil {
|
||||||
last = true
|
return netip.Addr{}, false
|
||||||
} else {
|
}
|
||||||
hop, value = value[comma+1:], value[:comma]
|
|
||||||
}
|
|
||||||
|
|
||||||
hop = strings.TrimSpace(hop)
|
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
|
||||||
if hop == "" {
|
return addr, true
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
addr, err := netip.ParseAddr(hop)
|
|
||||||
if err != nil {
|
|
||||||
return netip.Addr{}, false
|
|
||||||
}
|
|
||||||
|
|
||||||
if addr = normalizeAddr(addr); !m.isTrustedProxy(addr) {
|
|
||||||
return addr, true
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -143,10 +113,8 @@ 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, the most specific identity left. On a
|
// value rather than collapsing such peers into one
|
||||||
// Unix-socket listener every peer carries the same
|
// shared bucket.
|
||||||
// RemoteAddr and so shares one bucket, which is the
|
|
||||||
// fail-closed direction.
|
|
||||||
return r.RemoteAddr
|
return r.RemoteAddr
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -8,10 +8,7 @@ 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"
|
||||||
@@ -571,105 +568,6 @@ 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,4 +1,5 @@
|
|||||||
// 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