feat: debounce forced-reload trigger endpoints (PER-15248)#327
feat: debounce forced-reload trigger endpoints (PER-15248)#327dshoen619 wants to merge 3 commits into
Conversation
Replace OpalClient's ungated /policy-updater/trigger and /data-updater/trigger handlers (and route the legacy /update_policy* aliases) through per-updater DebouncedTrigger instances, so an authenticated caller or buggy SDK can no longer amplify full-reload load onto the shared control plane. Coalesces triggers within a configurable window (PDP_TRIGGER_DEBOUNCE_SECONDS, default 10s) and collapses concurrent triggers into the in-flight pull; a failed pull does not consume the window. Response/auth parity with the routes it replaces is preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔍 Vulnerabilities of
|
| digest | sha256:1544e28c4fade18f0b7300feea566516b6cd7e59d235863f9e8c2d396dbb05f1 |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 225 MB |
| packages | 264 |
📦 Base Image python:3.10-alpine3.22
| also known as |
|
| digest | sha256:c8f94b3bb77e6ea9015ccd091b7f8aec1b1fcbca95159675235d9a93788797cd |
| vulnerabilities |
Description
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
|
There was a problem hiding this comment.
Pull request overview
This PR adds per-updater debouncing/coalescing to the PDP’s forced-reload trigger endpoints to prevent authenticated callers (or buggy SDKs) from repeatedly forcing full control-plane repulls, and replaces OPAL’s trigger-route handlers with PDP-owned gated/debounced equivalents.
Changes:
- Introduces a
DebouncedTriggerutility to coalesce trigger calls within a configurable window and while a reload is in-flight. - Replaces OPAL-mounted
POST /policy-updater/triggerandPOST /data-updater/triggerroutes with PDP-owned,enforce_pdp_token-gated, debounced handlers; legacy aliases share the same debouncers. - Adds
TRIGGER_DEBOUNCE_SECONDSconfiguration (remote-config overridable) and new end-to-end behavior tests for coalescing semantics.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| horizon/debounce.py | Adds the debouncing/coalescing state machine for trigger calls (window + in-flight guard). |
| horizon/pdp.py | Removes OPAL trigger routes and re-registers PDP-owned debounced/gated replacements; wires in per-updater debouncers shared across canonical + legacy aliases. |
| horizon/config.py | Adds TRIGGER_DEBOUNCE_SECONDS (default 10s, 0 disables) to control debounce behavior. |
| horizon/tests/test_trigger_debounce.py | New integration-style tests validating within-window and in-flight coalescing, failure semantics, 0 disables, and canonical/legacy sharing. |
| horizon/tests/test_route_auth_audit.py | Updates regression message to reflect the new route replacement functions. |
| horizon/tests/test_opal_trigger_auth.py | Updates documentation to reflect route replacement (vs dependency injection). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…er-endpoints-to-dampen-reload
…lias) Addresses Copilot review comment on PR #327. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What & why
Closes PER-15248.
Even with auth enforced (PER-15244/45/46), a valid-token caller or a buggy SDK can hammer the forced-reload trigger endpoints, each of which forces a full re-pull from the shared control plane — a load-amplification vector across the fleet. This adds a small per-updater debounce so redundant/concurrent forced reloads coalesce instead of amplifying.
There are four amplifying routes, not two (the issue lists two): the OPAL-mounted
POST /policy-updater/triggerandPOST /data-updater/trigger, plus the PDP's legacy aliasesPOST /update_policyandPOST /update_policy_data, which call the same two updater methods directly. All four are now debounced.How
horizon/debounce.py—DebouncedTrigger: per-updater coalescing. Monotonic-clock window check; an in-flight guard that collapses concurrent triggers into the running pull regardless of the window (a full pull can run for minutes under a degraded control plane, so a time-window alone would still admit a concurrent pull every N seconds);_last_firedrecorded only on success, so a failed pull doesn't burn the window;None(never0.0) sentinel so the first trigger on a freshly booted host isn't silently coalesced.horizon/pdp.py: the OPAL-mounted handlers are closures we can't intercept, and a FastAPI dependency can't short-circuit to a200no-op — so we remove the two OPAL routes and re-register PDP-owned,enforce_pdp_token-gated, debounced replacements at the same paths (fail-loud if a path is missing, preserving the old guard's safety). The two legacy aliases share the same per-updater debouncers, so an alternating hammer still coalesces. This retires the previous_gate_opal_trigger_routesdependant-surgery.horizon/config.py:PDP_TRIGGER_DEBOUNCE_SECONDS(default10.0;0disables; remote-config overridable fleet-wide, so ops can raise it to 30–60s under a degraded control plane without a release).Decisions & scope
/kong(issue lists it as optional):/kongresolves against the local OPA cache (horizon/enforcer/api.py), so hammering it loads only that one PDP — no control-plane amplification, which is the entire threat model here. It also needs a different control (a genuine per-request rate limit), not a debounce that would return stale authz decisions. If local self-DoS on/kongis a concern, it should be its own issue.200from/data-updater/triggermeant the inline base-data fetch had completed; with debouncing a200now means "a recent or in-flight pull already covers you." Body stays exactly{"status": "ok"}so SDKs never error-spiral. The policy route already had fire-and-forget200semantics (queue put). A disabled data updater still returns503, checked before the debouncer so a503never consumes the window.Tests
New
horizon/tests/test_trigger_debounce.py(12 tests): within-window coalescing (policy + data + canonical/legacy sharing one debouncer), fires-again-after-window,0disables, in-flight coalescing past the window, failed pull doesn't burn the window,503-before-debouncer, per-updater independence, per-instance state, and auth still enforced. Full suite green (130 passed);ruff check+ruff format --checkclean.Based on
origin/mainbefore #326 (PER-15250) merged. #326 adds tests asserting the exact updater kwargs this change preserves (force_full_update=True,data_fetch_reason="request from sdk"/"…(legacy alias)"). Rebase onto main after #326 lands (minor docstring overlap intest_opal_trigger_auth.py) and re-run the gates before marking ready.Reviewed by a backend-architect pass over the diff (verdict: ship) confirming the in-flight guard has no interleaving
await, the success-only timestamp holds, and the debouncer is reachable only from the 4 routes (internal OPAL pubsub/reconnect resync bypasses it).🤖 Generated with Claude Code