feat(client): live liveness probe for opa policy store - #904
Merged
Conversation
OPAL client's /healthy endpoint previously delegated to the success of the last server -> policy-store transaction only. In steady state with no incoming updates, the flag stayed True even if OPA hung, deadlocked, or stopped accepting HTTP, causing supervisors that probe /healthy to miss the failure. Adds a long-lived background sampler on OpaClient (and CedarClient) that periodically GETs the policy store's health endpoint and updates an engine_reachable flag. /healthy now returns False within one probe interval when OPA is unreachable and recovers automatically when OPA returns. The endpoint itself remains O(1) (no per-request HTTP call), so high-frequency liveness polling does not translate into proportional load on OPA. New env vars (opt-out / tunable): - OPAL_POLICY_STORE_LIVENESS_PROBE_ENABLED (default True) - OPAL_POLICY_STORE_LIVENESS_PROBE_INTERVAL_SECONDS (default 10) - OPAL_POLICY_STORE_LIVENESS_PROBE_TIMEOUT_SECONDS (default 2) Public async signature of is_healthy() is unchanged. The same pattern is applied to CedarClient. Includes unit tests covering all four matrix cells (transactions ok/failed x engine reachable/unreachable) plus recovery.
✅ Deploy Preview for opal-docs canceled.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a background liveness sampler to OPAL Client policy-store clients so GET /healthy reflects live policy engine reachability (OPA/Cedar), not only the last successful server→store transaction.
Changes:
- Introduces configurable background liveness probing (enabled/interval/timeout) and wires probe start/stop into client lifecycle.
- Extends OPA and Cedar policy-store clients with periodic reachability sampling and folds results into
is_healthy(). - Adds unit tests for OPA liveness behavior and documents new configuration/env vars.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/opal-client/opal_client/tests/opa_client_liveness_test.py | New unit tests covering the liveness probe matrix and recovery behavior. |
| packages/opal-client/opal_client/policy_store/opa_client.py | Adds engine reachability flag into health computation and implements OPA liveness probe loop. |
| packages/opal-client/opal_client/policy_store/cedar_client.py | Implements analogous Cedar liveness probe and ANDs reachability into health result. |
| packages/opal-client/opal_client/policy_store/base_policy_store_client.py | Adds default no-op start_liveness_probe() / stop_liveness_probe() to the interface. |
| packages/opal-client/opal_client/config.py | Adds liveness probe enable/interval/timeout config options. |
| packages/opal-client/opal_client/client.py | Starts/stops policy-store liveness probe with client background task lifecycle; updates /healthy docstring. |
| documentation/docs/tutorials/healthcheck_policy_and_update_callbacks.mdx | Documents /healthy semantics and liveness probe behavior. |
| documentation/docs/getting-started/configuration.mdx | Adds configuration reference entries for the new env vars. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zeevmoney
approved these changes
Apr 28, 2026
Lifts the policy-store liveness probe lifecycle (start/stop/loop/session)
into a shared LivenessProbeMixin used by OpaClient and CedarClient. The
refactor removes near-duplicated code between the two clients and resolves
several issues called out in review on a single surface:
- Lock-guarded check-then-act in start_liveness_probe; second concurrent
call cannot spawn a second task.
- Single long-lived aiohttp.ClientSession per probe lifetime instead of
one per sample.
- task.add_done_callback that logs at ERROR if the loop ever exits with a
non-cancel exception.
- Cancel-only stop semantics (drop the redundant stop_event); the loop
uses asyncio.sleep(interval) which cancellation interrupts cleanly.
- Inner except limited to (ClientError, TimeoutError); a programming bug
no longer silently masquerades as "engine unreachable" — the loop's
outer except is the survival net and logs at WARNING.
- max(1, ...) floor on the configured timeout so TIMEOUT_SECONDS=0 cannot
disable the per-request bound.
- First probe runs synchronously inside start_liveness_probe so the
initial engine_reachable flag reflects reality, not the optimistic
default (matters for external-OPA where no runner has verified
reachability).
Other tightenings:
- OPA probe URL via removesuffix("/v1") after rstrip("/") instead of the
brittle [:-3] slice.
- Probe sends no auth headers — OPA /health is unauthenticated by
default; this decouples reachability sampling from the OAuth token
issuer.
- OpaTransactionLogState.healthy logs at DEBUG (not WARNING) since
/healthy can now be polled at high frequency while the engine is down;
the probe loop already logs INFO on transitions.
- launch_policy_store_dependent_tasks logs probe-start failure at ERROR
with an explicit message ("/healthy will not reflect engine
reachability") instead of a bare exception trace.
Addresses review comments:
- #904 (comment) (@copilot-pull-request-reviewer)
- #904 (comment) (@copilot-pull-request-reviewer)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drops the racy _find_free_port helper for servers under our control:
the toggle server now binds aiohttp to port 0 and reads the assigned
port back from the started site. The connection-refused test still
needs an explicitly-unbound port and uses a renamed helper with a
comment documenting that the race is theoretical there.
- Replaces deprecated asyncio.get_event_loop().time() with
time.monotonic() in test polling helpers.
- Adds OPA tests for: start_liveness_probe idempotency (same task object
on a second call) and OAuth-irrelevance (a probe configured against an
unreachable OAuth IdP still reports the engine reachable, since the
probe sends no auth headers).
- Adds a CedarClient liveness suite mirroring the OPA shape: reachable,
5xx, and recovery. The Cedar probe URL (/v1/) matches
CedarRunner.health_check()'s precedent; the new tests pin the
end-to-end behavior so a future regression on the Cedar side is caught.
A FastAPI route-level test (TestClient(app).get("/healthy") asserting a
503 once the probe trips) was considered and deferred — it would require
substantially more scaffolding (full OpalClient + app + server) and the
underlying signal is already covered by the policy-store-level tests
here.
Addresses review comments:
- #904 (comment) (@copilot-pull-request-reviewer)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
- #904 (comment) (@zeevmoney)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-commit hooks reformatted these files in CI but the changes weren't included in the prior two commits. No behavioral changes: black collapses multi-line function signatures that fit within the line length, and docformatter re-wraps docstrings to its width limit. CI fixes: - pre-commit: black + docformatter hooks were modifying files, causing the workflow to exit non-zero. Re-ran both at the pinned versions (black 23.1.0, docformatter 1.7.5) locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OPAL Client's
GET /healthypreviously returned 200 as long as the last server -> policy-store transaction succeeded. In steady state with no incoming updates, the flag stayedTrueeven if OPA hung, deadlocked, dropped its listener, or stopped accepting HTTP. Supervisors that probe/healthy(e.g. Permit's PDP Rust watchdog) therefore never tripped, and the PDP could not serve/v1/data/...queries until manual intervention.This PR adds a long-lived background sampler on
OpaClientthat periodicallyGETs{POLICY_STORE_URL}/healthand feeds the result intois_healthy(), so/healthyreflects live OPA responsiveness, not just the last transaction. It also applies the same pattern toCedarClient.Failure mode being fixed
is_healthy()now returnsFalsewithin one probe interval when OPA is unreachable (connection refused, timeout, non-2xx on/health) and recovers toTrueautomatically when OPA returns — no OPAL Client restart required.Why a background sampler over a per-request probe
/healthyis hit by k8s probes / watchdogs at high frequency (often every 1–5s). Doing a synchronous HTTP call to OPA on every/healthyrequest would:/healthylatency to OPA latency — a slow OPA would slow down the very probe meant to detect that.A single background sampler decouples probe cost from
/healthyQPS:/healthystays O(1) and OPA sees one request per interval regardless of supervisor polling.Initial value of
_engine_reachableChosen:
Trueuntil the first probe completes.Rationale: the engine runner already health-checks OPA before
launch_policy_store_dependent_tasksis called (seeOpaRunner._wait_for_engine_health). At the moment we start the sampler, OPA is known-good. Starting fromTrueavoids a brief window of false-unhealthy at startup, and matches the historical behavior immediately after launch. Documented inline next to the field.Cedar
Same pattern applied to
CedarClient(probes{POLICY_STORE_URL}/v1/). Tested at the unit level for OPA; Cedar shares the same lifecycle wiring throughBasePolicyStoreClient. Worth a smoke-test in a Cedar deployment, but no breaking change.What's in this PR
opal_client/config.py)OPAL_POLICY_STORE_LIVENESS_PROBE_ENABLED(defaultTrue)OPAL_POLICY_STORE_LIVENESS_PROBE_INTERVAL_SECONDS(default10)OPAL_POLICY_STORE_LIVENESS_PROBE_TIMEOUT_SECONDS(default2)OpaClientOpaTransactionLogStategainsengine_reachable(defaultTrue), ANDed into.healthyso the rego healthcheck policy (system.opal) also reflects live engine reachability._probe_engine_reachable()— single GET to{POLICY_STORE_URL}/health, honorsPOLICY_STORE_AUTH_*(bearer / OAuth) and TLS context._liveness_probe_loop()— INFO logs only on transitions, DEBUG for steady-state samples; graceful shutdown viaasyncio.Event.start_liveness_probe()/stop_liveness_probe()— idempotent.BasePolicyStoreClient— default no-opstart_liveness_probe/stop_liveness_probeso non-supporting stores remain unaffected.OpalClient(client.py)TODOnext to/healthy.launch_policy_store_dependent_tasksafter the engine is up; cancels it instop_client_background_tasks.CedarClient— analogous probe targeting/v1/.configuration.mdxand a call-out in the healthcheck-policy tutorial.async def is_healthy(self) -> boolsignature unchanged. No breaking change.Acceptance criteria check
Falsewithin one probe intervalTrueagain without restart/healthyremains O(1); high-freq polling doesn't load OPA/healthyonly reads a flagOPAL_POLICY_STORE_LIVENESS_PROBE_ENABLED(defaultTrue)TODOinclient.pyremovedTest plan
pytest packages/opal-client/opal_client/tests/opa_client_liveness_test.py— 7 tests pass (covers all four matrix cells + connection refused + probe-disabled + idempotent stop)opa_client_test.pyandengine_runner_test.pystill pass (19 tests, no regressions)OPAL_POLICY_STORE_LIVENESS_PROBE_ENABLED=True,kill -STOPthe OPA process, observe/healthyflip to 503 within ~10s, thenkill -CONTand observe recovery.OPAL_POLICY_STORE_LIVENESS_PROBE_ENABLED=Falseconfirms historical behavior is preserved./healthyflips when Cedar is killed.Out of scope
ready.