feat(connectors): add the HTTP source webhook gateway connector - #3798
feat(connectors): add the HTTP source webhook gateway connector#3798mlevkov wants to merge 5 commits into
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3798 +/- ##
============================================
+ Coverage 83.87% 84.04% +0.17%
Complexity 1358 1358
============================================
Files 1212 1220 +8
Lines 166843 170222 +3379
Branches 134306 137810 +3504
============================================
+ Hits 139937 143064 +3127
- Misses 23266 23343 +77
- Partials 3640 3815 +175
🚀 New features to boost your workflow:
|
504c603 to
ebca5c2
Compare
|
/request-review @hubcio |
|
Pushed a follow-up commit closing the coverage gaps worth closing. Kept it as a Patch coverage was 94.52% (2847 hits / 131 misses / 34 partials). Reading the Both existing management auth tests reached only Also newly pinned: Every new test was mutation-checked — each was confirmed to fail against a One production line changed: Deliberately left uncovered, and why:
Full local gate green (fmt, sort, workspace clippy |
4c60da8 to
6d3bcac
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you need a review, please ensure CI is green and the PR is rebased on the latest master. Don't hesitate to ping the maintainers - either Thank you for your contribution! |
6d3bcac to
a358ddf
Compare
Iggy has no way to receive a webhook. Every provider that pushes events over HTTP needs something in front of it, and today that means running a separate service whose only job is to accept a POST and republish it. This connector removes that hop: it runs an embedded HTTP server, accepts authenticated POST bodies, and produces them to the instance's stream and topic as raw bytes. One plugin .so is loaded once no matter how many source entries reference it, so the listener cannot live on any single instance. It lives in a process-global registry keyed by listen address: the first open binds the public and admin ports, later opens validate their body limit, admin address, management token and instance name against the running listener before joining, and the last close releases both ports. Mismatches fail that instance's open rather than silently handing it a listener its configuration does not describe. A single port can therefore serve many providers, each routed to its own topic. Requests resolve against an ArcSwap route table that is rebuilt whole on every control-plane change, so one atomic load yields both the endpoint's auth rules and the destination bridge. Secret paths carry 128 bits in the URL itself, on the model of a Slack webhook, with optional bearer or HMAC on top; HMAC is verified over the raw body in constant time. Revoked endpoints answer 404 alongside paths that never existed, so a leaked URL cannot be used to confirm it was once live. Endpoints can be registered, re-keyed and revoked at runtime through a token-guarded API on the admin listener, because revoking a compromised endpoint is time-critical and provisioning one per tenant is inherently programmatic. Those endpoints ride the SDK's ConnectorState, and state is attached only to an empty batch: the runtime saves state solely on the success branch of the Iggy send, and an empty send always succeeds, so a mutation cannot be lost to an unrelated send failure. Revocation writes a tombstone that outranks TOML on restore, so a stale config file cannot resurrect an endpoint an operator revoked. Delivery is best-effort in both directions and the README says so first, before anything else: HTTP 200 means accepted into an in-memory buffer, and both the loss and duplicate windows are enumerated with what mitigates each. A full bridge answers 429 with Retry-After rather than blocking, since holding the connection open would turn a slow Iggy into a retry storm. Gateway metrics on the admin listener cover accept-to-200 latency, which the runtime's own stage histograms begin too late to see. Part of the webhook gateway design accepted in apache#3039. The backpressure chain is only complete once the bounded runtime forwarding channel from apache#3795 lands; until then a full bridge signals an arrival burst rather than a slow Iggy, which the README documents. Co-authored-by: Claude <noreply@anthropic.com>
The 94.5% patch coverage on this branch hid the gap that mattered: both management auth tests reached only GET /admin/endpoints, so the guard on each of the four mutating routes had never taken its rejection branch. Removing `denied()` from `revoke_endpoint` left the suite green, which a per-route table-driven test now catches at 204-instead-of-401. The rest closes the branches a reviewer would want pinned rather than every uncovered line. Chief among them, `republish_or_close` had no test proving it answers 500 instead of reporting a revoke that never reached the route table, and the endpoint-id route conflict rendered its message without any test asserting the id stays an 8-char prefix. `ServerState::new` widens to `pub(crate)` so the management tests can build a listener-less state and provoke the republish failure. Every new test was mutation-checked: each one was confirmed to fail against a deliberate break of the behaviour it claims to pin. Left uncovered on purpose: the shutdown abort branch needs a connection wedged past the 5s timeout, and roughly 30 of the remaining missed lines are tracing-macro arguments that only execute with a subscriber installed.
a358ddf to
50f8258
Compare
The bridge drain in `poll()` is destructive, so the delivery result added by apache#3855 had nothing to act on here: a NACK left the events nowhere at all, even though the runtime expects the next poll to produce them again. `on_batch_result` has a default no-op, so this compiled and stayed quiet. The batch is now held until the runtime acks it and replayed otherwise, which is what makes the connector recover a failed send instead of absorbing it. Nothing is ever abandoned. Answering 200 already told the sender this gateway owns the event, and the only honest way to shed load is the 429 the handlers return once the bridge fills, which senders retry. Dropping a staged batch would trade that bounded, visible backpressure for silent loss growing with the length of the outage. The usual argument for a give-up bound does not apply: oversized bodies are rejected with 413 before a handler runs, headers are clamped on accept, and `Schema::Raw` cannot fail to decode, so a permanently undeliverable batch is not reachable from the accept path. What remains is the SDK stopping a source after five consecutive NACKs, around 1.5s of backoff plus five send rounds, which is the right call for a source that can re-read its cursor and the wrong one for a buffer that cannot be replayed. That belongs in the SDK rather than in a workaround here, and the README now states it as the residual window. Two existing poll tests asserted the pre-ack behaviour by polling twice without acknowledging, and now ack between the two.
The residual window described the SDK breaker without saying it is tracked, and left out the part that makes it dangerous: the poll task ends while the runtime's forwarding loop stays parked, so the running count keeps counting a source that no longer polls.
|
Rebased onto master and followed up on
Nothing is ever abandoned. Answering 200 already told the sender this gateway owns the event, and the only honest way to shed load is the 429 the handlers return once the bridge fills, which senders retry. Dropping a staged batch would trade that bounded, visible backpressure for silent loss that grows with the length of the outage. The usual case for a give-up bound does not apply here either: oversized bodies are rejected with 413 before a handler runs, headers are clamped on accept, and Two existing poll tests were asserting the pre-ack behaviour by polling twice without acknowledging, and now ack between the two. Four new tests cover replay on NACK, release on ACK, an empty state-only batch not staging a replay, and the never-abandon property itself so it cannot be quietly reversed. One thing that needs a decision outside this PR: #3941. The SDK stops a source after five consecutive NACKs, which is about 1.5s of backoff plus five send rounds, so roughly two seconds of broker unavailability ends the poll task. That is correct for a source that can re-read its cursor and wrong for this one, whose bridge is in memory: the listener keeps accepting, the bridge fills to There is a loophole that would let a plugin survive this (an empty batch always acks, which resets the breaker's counter) and I have deliberately not used it, because it defeats an SDK safety mechanism from inside a plugin. The README states the residual window and points at #3941 rather than working around it here.
|
MD013 caps a line at 500 characters and folding the breaker into the producer-failure entry pushed it to 871. It reads better split anyway: producer failure is now recovered by the replay, and what remains is the poll task being stopped, which is a separate window with a separate cause.
|
/request-review @hubcio |
1 participant
Implements the webhook gateway accepted in #3039. Iggy currently has no way to receive a webhook; every provider that pushes events over HTTP needs a separate service in front whose only job is to accept a POST and republish it. This connector removes that hop.
Shape
One plugin
.sois loaded once regardless of how many[[source]]entries reference it, so the listener lives in a process-global registry keyed by listen address rather than on any single instance. The firstopen()binds the public and admin ports; later opens validate their body limit, admin address, management token and instance name against the running listener before joining; the lastclose()releases both ports, which the runtime's stop-then-start restart flow depends on. A single port can therefore serve many providers, each routed to its own topic.POST /topics/{topic_path}— named path, one per instance, guarded by an optional bearer tokenPOST /e/{endpoint_id}— secret path, 128 bits in the URL itself, with optional per-endpoint bearer or HMAC on topRequests resolve against an
ArcSwaproute table rebuilt whole on every control-plane change, so one atomic load yields both the endpoint's auth rules and the destination bridge. HMAC is verified over the raw body in constant time. Revoked endpoints answer 404 alongside paths that never existed, so a leaked URL cannot be used to confirm it was once live.Endpoints can be registered, re-keyed and revoked at runtime through a token-guarded API on the admin listener (absent entirely when no token is configured). Those endpoints ride the SDK's
ConnectorState, and revocation writes a tombstone that outranks TOML on restore, so a stale config file cannot resurrect an endpoint an operator revoked.Delivery semantics
Best-effort in both directions, and the README leads with it rather than burying it. HTTP 200 means accepted into an in-memory buffer, not durably stored; both the loss and duplicate windows are enumerated with what mitigates each. A full bridge answers 429 with
Retry-Afterrather than blocking.Depends on #3795 for the full backpressure story. The bridge is bounded today, so 429 fires on an arrival burst the poll loop cannot keep up with. What is missing is the coupling: until the bounded runtime forwarding channel lands,
poll()drains into an unbounded channel, so a slow Iggy does not propagate back into 429. The README documents this rather than implying the chain is complete.State is attached only to an empty batch. The runtime saves state solely on the success branch of the Iggy send, and an empty send always succeeds, so a management mutation cannot be lost to an unrelated send failure.
Deviations from the design doc, all deliberate
topic_pathandinstance_nameare explicit config fields. Onlyplugin_configcrosses the FFI, so the plugin cannot see its connector key or its[[streams]]entry. Same resolution the design already accepted for the named path.GET /healthwhen no instance is joined.{timestamp}.{body}behind a compound header and Twilio signs URL plus sorted params as base64. The design's example config showed Stripe working; it would not have. Documented with the forward-and-verify-downstream workaround, and the shipped example uses bearer instead.schema = "raw"is mandatory and now stated as such — the connector always produces raw bodies, and a JSON encoder rejects every message.Testing
110 unit tests and 5 integration tests. The integration suite needs no containers, since the connector is itself the HTTP server and the test client is the webhook sender: it covers a signed POST reaching Iggy byte-for-byte with headers intact, two instances sharing one listener, the register/POST/revoke flow, a dynamic endpoint with its secret surviving a connector restart, and a revoked static endpoint staying dead across a restart that re-reads the TOML still declaring it.
No integration test for 429 under saturation: with a healthy Iggy the poll loop drains the bridge continuously, so provoking a full bridge from outside races the drain. Two unit tests cover it deterministically instead.
Verification
cargo fmt,cargo sort --check --no-format --workspace,cargo clippy --all-features --all-targets -- -D warnings(connector and integration),cargo test,taplo fmt --check,hawkeye check, markdownlint, and the trailing-whitespace/newline scripts all pass locally.