Add New Edge PDP documentation - #649
Conversation
Documents the new single-binary PDP (permitio/pdp-v3) as an additional deployment option alongside the existing Edge PDP (permitio/pdp-v2), which remains fully supported. Covers the architecture (embedded SurrealDB/RocksDB, supervised OPA and NATS leaf children, loopback query/bundle servers), how policy and data stay in sync over a durable per-PDP event stream, the consistency model, cold start vs warm resume, resource footprint and tuning, deployment requirements, the security model, and the feature-support envelope. Every endpoint, environment variable, port, and default in this page was verified against the cloud-pdp source rather than the repo's README or runbook, both of which are stale in several places. Three points are deliberately stated as gaps rather than features, because the implementation contradicts them today: - OpenTelemetry is not wired up in edge-pdp. It never calls init_observability, has no opentelemetry dependency, reads no OTEL_* vars, and serves no /metrics endpoint. - Absolute resource footprint is not reduced. CI pins a 4GiB limit and notes that 500 MiB OOMKills. The documented win is architectural: the fact graph moves from OPA's in-memory document to on-disk RocksDB, so memory stops scaling with data set size. - No propagation-latency or throughput figures exist in the repo, so the page makes the structural argument and explicitly says numbers are not yet published. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
✅ Deploy Preview for permitio-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
🟡 Changes recommended
The new documentation has internal inconsistencies/ambiguities (notably health endpoint naming and PDP_CONTROL_PLANE format wording) that should be corrected to prevent misconfiguration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds documentation for the new single-binary New Edge PDP (permitio/pdp-v3) as an additional, non-replacing deployment option alongside the existing Edge PDP.
Changes:
- Adds a new MDX page detailing architecture, sync/consistency model, feature coverage, configuration, and operational requirements for the New Edge PDP.
- Registers the new page in the PDP section of the Docusaurus sidebar navigation.
File summaries
| File | Description |
|---|---|
| sidebars.js | Adds the New Edge PDP doc to the PDP docs sidebar list. |
| docs/concepts/pdp/new-edge-pdp.mdx | New documentation page for the New Edge PDP runtime, endpoints, configuration, and deployment guidance. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The Mermaid diagram rendered small and could not be clicked. Docusaurus renders Mermaid as inline SVG, so it is capped at the ~640px doc content column and the image-zoom plugin never applied to it — that plugin is wired to `.markdown :not(em) > img`, which only matches real <img> elements. Replaces it with the architecture SVG extracted from the source diagram, served as a static asset. It now renders as an <img>, so click-to-zoom applies and the diagram opens near full viewport width where every port, label and edge is legible. Verified in a browser: medium-zoom attaches and the zoomed state renders correctly. The SVG relied on CSS custom properties defined by its original host page, so the extracted file inlines the palette, paints its own background, and carries a prefers-color-scheme dark variant — otherwise it would render colourless, or as dark text on a dark page. Fonts fall back to a system monospace stack rather than the webfont the original pulled from Google, which cannot load inside an <img>. Also renames the container frame label to "New Edge PDP" and widens its backing mask to match, so the dashed region border stays hidden behind the longer string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟡 Changes recommended
The new documentation has a couple of internal inconsistencies/mislabeling issues (notably health endpoint naming and accessible diagram titling) that should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
docs/concepts/pdp/new-edge-pdp.mdx:246
- The Feature support table lists
GET /healthy, but the Observability section defines readiness asGET /health/readyon the management port. This looks like a typo/inconsistency within the page; align the supported endpoint list with the later section.
| Health and readiness | `GET /health`, `GET /healthy` |
- Files reviewed: 2/3 changed files
- Comments generated: 1
- Review effort level: Lite
…text Three findings from the Copilot review on #649. Health endpoints (valid concern, different cause than reported). Copilot read `GET /healthy` and `GET /health/ready` as an internal contradiction. Both are real, on different listeners: the authorization port serves /health and its /healthy alias, while /health/ready and /health/detail exist only on the management port. Rather than drop either, the page now says so explicitly and warns about the two ways this bites — a readiness probe on :7000/health/ready 404s forever, and :7000 does not open until the PDP is ready, so any probe there fails for the whole cold start and a liveness probe would kill the container before it finishes booting. PDP_CONTROL_PLANE. The text said "bare host and port" while the example carried a wss:// scheme. Now states scheme + host + explicit port, no path, no trailing slash. Diagram alt text said "Edge PDP container"; now "New Edge PDP". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-only and structurally sound, with only a minor accessibility metadata tweak suggested for the new SVG.
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
| :::caution Cold start is not fast — plan probes accordingly | ||
| A cold start transfers and loads the environment's entire data set. Permit's own CI observes **84 seconds to roughly 4 minutes** for this on test environments, and larger environments will take longer. | ||
|
|
||
| The health endpoint answers throughout, so **set a generous startup probe** and let readiness — not liveness — gate traffic. The authorization port does not open until the PDP is ready, so there is no window in which it accepts a request it cannot answer correctly. | ||
| ::: |
There was a problem hiding this comment.
where did you take this data from ? the new edge pdp has faster coldstart then the old one
There was a problem hiding this comment.
From .github/workflows/run_edge_pdp_tester.yml:146 in cloud-pdp — START_TIMEOUT: "900" # cold boot 84s-4min draining the snapshot, alongside the job comment at line 60 saying "cold boot alone can take 4 min per PDP". So it was a CI observation, not a comparison against v2 — which is exactly the problem, since quoted with no baseline it reads as a criticism.
Rewritten. It now says bulk-loading pre-built database files is substantially faster than v2's cold start, which applies its data set through the normal write path, and the CI figure is gone. I kept the startup-probe guidance, since that's the part readers actually act on.
If there's a published v2-vs-v3 cold-start number I can cite, I'll add it — that would be a much stronger claim than the qualitative one.
| :::note What this does and does not guarantee | ||
| The consistency model is **eventual, with a convergent merge**. It guarantees that PDPs converge, that no update is lost, and that replays are safe. | ||
|
|
||
| It does **not** provide read-your-own-writes. There is no local write API — the New Edge PDP is read-only with respect to your data, and a fact written through Permit's API becomes visible once it propagates. If you need write-then-immediately-read semantics, use the existing Edge PDP's [local facts uploader](/how-to/manage-data/local-facts-uploader). |
There was a problem hiding this comment.
this might be a feature we'll add in the future, but don't commit to we adding it at some point or provide a timeline
There was a problem hiding this comment.
Tightened. Removed the phrasings that read as commitments — "known gap rather than a design decision", "not available yet" — so the only forward-looking signal on the page is the 🚧 marker in the matrix, with no timeline attached anywhere. Read-your-own-writes is now stated as current behaviour only, pointing at the local facts uploader as the option that exists today.
| | **RBAC** | — | | ||
| | **ReBAC**, including role derivation | — | | ||
| | **Multi-tenancy** | — | | ||
| | [Check](/how-to/enforce-permissions/check) | `POST /allowed` | |
There was a problem hiding this comment.
these are basically a part of each check, it should be some sort of nested table of
Check
- RBAC
- ReBAC ...
There was a problem hiding this comment.
Done — RBAC, ReBAC and multi-tenancy are now nested under Check with a └ indent rather than sitting as peers of it, and ABAC joins them as the one that is not supported. That reads much better; they were misleading as top-level rows.
| | Decision logs to Permit | — | | ||
| | Health check on the authorization port | `GET /health`, `GET /healthy` (aliases for the same handler) | | ||
|
|
||
| ### Not supported |
There was a problem hiding this comment.
I think it's better to create a matrix of feature compatibility and roadmap
There was a problem hiding this comment.
Done. The supported/not-supported split is replaced by a single compatibility matrix scoring each capability against both runtimes, with a ✅ / 🚧 roadmap / ❌ key, grouped into Permission checks, Standards, Policy and data, and Operations.
Building it surfaced two errors in my original lists, both now fixed:
- I had Envoy ext-authz as a New Edge PDP gap, but v2 doesn't have it either — dropped rather than shown as a regression.
- Prometheus
/metricsand OTLP export are now separate rows. v2 has the former (horizon/config.py:250) but not the latter, so collapsing them would have got both columns wrong.
| :::caution OpenTelemetry is not available yet | ||
| The New Edge PDP **does not currently export OpenTelemetry traces, metrics, or logs**, and does not read `OTEL_*` environment variables. It also does not serve a `/metrics` endpoint. | ||
|
|
||
| Observability today is structured stdout logs plus the health endpoints described above. If OTLP export is a requirement for your deployment, [let us know](mailto:support@permit.io) — it is a known gap rather than a design decision. | ||
| ::: |
There was a problem hiding this comment.
this is a part of the feature parity table ( old pdp doesn't have it, new is in the roadmap )
There was a problem hiding this comment.
Moved into the compatibility matrix under Operations — OTLP export is ❌ for the Edge PDP and 🚧 roadmap for the New Edge PDP, with Prometheus /metrics as its own row since v2 does have that one. The Observability section keeps a short factual note pointing at the matrix, without the "known gap" phrasing.
| | Variable | Description | | ||
| | --- | --- | | ||
| | `PDP_API_KEY` | Your environment-level Permit API key. This single value binds the container to one environment, authenticates it to the control plane, and is the bearer token the authorization API accepts. | | ||
| | `PDP_CONTROL_PLANE` | The control-plane endpoint. **See the warning below — this is not the same kind of value as on `pdp-v2`.** | |
There was a problem hiding this comment.
there is no need to configure this anymore, the apikey of the v3 automatically routes the PDP to the correct plane
There was a problem hiding this comment.
Removed — good catch, and it would have been an actively harmful instruction.
Confirmed in the code too: edge-pdp/src/config.rs:543 describes PDP_CONTROL_PLANE as an "operator ESCAPE HATCH, not the normal path", with the bundle's own nats_url winning by default. The page now says the API key carries the routing and there is no endpoint URL to configure.
| | `EDGE_DB_PATH` | `/var/lib/edge-pdp/db` | Embedded database directory. **Must be on a persistent volume.** | | ||
| | `EDGE_DATA_DIR` | `/var/lib/edge-pdp` | Event store and generated credentials. **Must be on a persistent volume.** | | ||
| | `EDGE_PUBLIC_PORT` | `7000` | Authorization API. The only port your services need. | | ||
| | `EDGE_HEALTH_PORT` | `7001` | Health and readiness. | |
There was a problem hiding this comment.
fyi - some env vars might change here in the future
There was a problem hiding this comment.
Noted — the new configuration page carries an early-access callout saying variables and defaults may change before GA, and advising readers to pin an image tag and re-check on upgrade. Happy to mark specific variables as unstable if you know which ones are most likely to move.
There was a problem hiding this comment.
Followed up on this — rather than caveat a reference that's likely to go stale, we've pulled the environment variable reference out entirely for now. The configuration page is gone and the section on the main page keeps only PDP_API_KEY, pointing at support for anything else.
The operational guidance survives without the variable names: persistent storage is still mandatory, terminationGracePeriodSeconds is still stated as ≥40s (as a combined shutdown budget rather than naming the two vars), and the warning that turning up child log levels exposes authorization data is still there.
Easy to add back once the surface settles.
There was a problem hiding this comment.
It might be beneficial to split some parts here to a different page, e.g configuration as we already have a configuration page for the old one
There was a problem hiding this comment.
Done — configuration is now its own page at /concepts/pdp/new-edge-pdp-configuration, mirroring the existing /concepts/pdp/configuration, and added to the sidebar under the PDP section. The main page keeps a short pointer plus the one variable that actually matters on day one (PDP_API_KEY). That also pulled the storage-engine tuning table off the main page. Main page is down from ~436 to ~400 lines with the reference material split out.
There was a problem hiding this comment.
I removed the page for now
There was a problem hiding this comment.
the configuration page does not appear on the sidebar + this page is still super long, and need some more splitting
There was a problem hiding this comment.
Both handled.
Sidebar — the configuration page has since been removed entirely (it was pulled after your env-var comment, since the surface is still moving). The four pages that exist now are all wired in, as a collapsible New Edge PDP sub-category under the PDP section with the overview as its landing page. Verified in a built site: the breadcrumb renders The Policy Decision Point (PDP) › New Edge PDP › Feature Parity.
Length — split from one ~380-line page into four:
- Overview — what it is, why, architecture, choosing between the two
- How It Works — sync, consistency, latency, cold start, scale, footprint
- Feature Parity — the comparison table
- Deployment — requirements, observability, security model
Happy to split further if any of them still reads long — How It Works is the biggest at ~155 lines.
There was a problem hiding this comment.
we should have configuration page, it should be marked as beta and open for changes but it is something we need
There was a problem hiding this comment.
Flagging rather than actioning this one, because it reverses an explicit call by the PR author.
There was a configuration page earlier in this PR. After your note that some env vars are likely to change, the author asked for it to be removed rather than shipped with a caveat — first the variable tables, then the section entirely. So restoring it is their decision, not mine to quietly undo, and I have put your request to them.
Worth saying I think your framing resolves the original objection: a page marked beta and explicitly open to change is a different proposition from a reference that silently goes stale. If the author agrees, it comes back quickly — the content still exists in this branch's history at 054aaec^ and can be restored with the beta callout added.
Note the current pages do not leave readers with nothing: PDP_API_KEY is documented as the only required value, and the operational settings that matter (persistent storage, ≥40s terminationGracePeriodSeconds, probe targets and ports, memory sizing) are all on the Deployment page — just described rather than pinned to variable names.
There was a problem hiding this comment.
Done — the configuration page is back, marked beta, and in the sidebar under New Edge PDP → Configuration.
Your framing is what made it work. It opens with a caution stating the surface is not stable, that variables may be renamed, replaced or removed before GA, that it should be treated as a working reference rather than a contract, and that readers should pin an image tag and re-read on upgrade — plus an ask to tell us if they are depending on a specific variable so we can flag it before it moves.
Restored from before the deletion and brought up to date: "container PDP" throughout, and deep links into pages that did not exist when it was first written (the port table on Architecture, health-and-readiness and deployment-requirements on Deployment, resource-footprint on How It Works). The storage-engine tuning advice now points here instead of at support.
Covers credentials, storage and ports, behavior and tuning, child-process log levels with the data-exposure warning, and the storage-engine knobs.
…l-plane var Restructures per review feedback on #649. Feature compatibility and roadmap matrix replaces the split supported/not-supported lists. RBAC, ReBAC, multi-tenancy and ABAC are now nested under Check rather than floating as peers of it, and every row is scored against both runtimes with a supported / roadmap / not-supported key. PDP_CONTROL_PLANE is gone from the docs. The API key carries the routing and sends the PDP to the correct plane on its own; edge-pdp/src/config.rs:543 calls the variable an "operator ESCAPE HATCH, not the normal path", so documenting it as required was wrong and would have had customers setting a value they do not need. Cold start no longer leads with "is not fast" or quotes the CI figure. It now says what is actually true and useful — bulk-loading pre-built database files beats the old PDP's cold start, which applies its data set through the normal write path — while keeping the startup-probe guidance, which is what readers act on. Configuration moves to its own page, mirroring the existing PDP's configuration page, and carries an early-access note that variables and defaults may still change. The main page keeps a short pointer. OpenTelemetry moves into the matrix as roadmap. Removes forward-looking phrasing elsewhere ("known gap rather than a design decision", "not available yet") so nothing reads as a commitment or a timeline. Also corrects two rows while building the matrix: the old PDP has no Envoy ext-authz either, so listing it as a New Edge PDP gap was misleading and it is dropped; Prometheus /metrics and OTLP export are now separate rows, since the old PDP has the former (horizon/config.py:250) but not the latter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟡 Changes recommended
There are a few user-facing documentation/accessibility inconsistencies (e.g., “single-binary” wording vs documented child processes, and an SVG title mismatch) that should be corrected before publishing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
docs/concepts/pdp/new-edge-pdp.mdx:10
- This tagline repeats “single-binary”, but the document itself describes separate child processes (OPA + NATS leaf). Rewording to “single-container” keeps it consistent with the Architecture section.
**A single-binary Policy Decision Point with an embedded database, kept in sync over a durable event stream.**
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
| --- | ||
| title: New Edge PDP | ||
| sidebar_label: New Edge PDP | ||
| description: A single-binary PDP with an embedded database that keeps a local copy of your policy and authorization data in sync over a durable event stream — no network in the decision path. |
There was a problem hiding this comment.
Fair — it does contradict the architecture table two sections down, which lists OPA and the NATS leaf as supervised child processes.
I went a different way than "single-container", though: the container PDP is also a single container, so that phrasing would trade a contradiction for a claim that distinguishes nothing. Both frontmatter and the opening line now lead with what actually is distinctive — an embedded on-disk database, kept in sync over a durable event stream, no network in the decision path. The precise in-process-vs-child breakdown stays in the architecture table, which was already correct.
While fixing it I also dropped "fewer moving parts — one binary" from the comparison table. Both runtimes run three processes (Python API server + OPAL client + OPA, against Rust binary + NATS leaf + OPA), so that was simply false. It now reads "a compiled runtime, with no Python or OPAL layer".
Fixed in 984c38a.
Omer flagged that some env vars are likely to change. Rather than publish a reference that goes stale, this removes it — it can come back once the surface settles. Deletes the configuration page added in the previous commit and its sidebar entry. The Configuration section on the main page keeps only PDP_API_KEY, which is the credential and is not going to change, and points readers at support for anything else. Also strips the volatile EDGE_* and SURREAL_ROCKSDB_* names from the prose that mentioned them incidentally, keeping the guidance itself intact: persistent storage is still mandatory, the 40s grace period is still stated as a combined shutdown budget, the child-process log-level warning still warns that turning them up exposes authorization data, and the storage-engine tuning advice still says which knobs matter without naming them. RUST_LOG stays, being a Rust ecosystem convention rather than something this PDP defines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟢 Approval recommended
Changes are documentation-focused and low-risk, with only a minor accessibility labeling fix suggested for the new SVG asset.
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Drops the whole section rather than keeping a stub. Configuration is settling and onboarding for the New Edge PDP goes through Permit anyway, so a partial section was carrying little and risked going stale. Nothing else depended on it: no inbound links or anchors pointed at it, and the operational requirements readers actually need — persistent storage, the 40s grace period, probe targets, memory sizing, which ports to expose — all live in Deployment requirements. PDP_API_KEY still appears where it is describing behaviour rather than instructing configuration: the bearer for /health/detail, and the constant-time comparison in the security model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🔵 Needs a closer look
The new diagram/page introduce small but user-facing documentation/accessibility inconsistencies (SVG title naming and “click to enlarge” behavior) that should be corrected before publishing.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
docs/concepts/pdp/new-edge-pdp.mdx:42
- The caption says “Click the diagram to enlarge it”, but the image is not linked or wrapped in the site’s zoom component, so there’s no explicit click-to-enlarge behavior.
- Files reviewed: 2/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
would rephrase the old PDP from the term Edge PDP as it is mostly confusing and unused term until now
There was a problem hiding this comment.
Agreed — "Edge PDP" for v2 is gone from all these pages. It is now the container PDP (permitio/pdp-v2), which matches the wording already used in cloud-pdp-capabilities.mdx. "New Edge PDP" stays as the name for v3, per the naming decision on this PR.
There was a problem hiding this comment.
the configuration page does not appear on the sidebar + this page is still super long, and need some more splitting
| | ABAC, condition sets, user or resource sets | The largest data sets, without sharding | | ||
| | Policy-as-Code / custom Rego bundles | Bounded, tunable memory rather than memory that scales with data | | ||
| | Read-your-own-writes / local facts | Decisions that never depend on reaching Permit's API | | ||
| | Data filtering via partial evaluation | Sync that survives long disconnections without a re-fetch | |
There was a problem hiding this comment.
this is not a feature even on the old one, remove it
There was a problem hiding this comment.
Removed — and you're right in both columns, so this was worse than a stray row.
Checked the v2 source: there's no compile/partial-evaluation endpoint in horizon/ or pdp-server/. So scoring partial evaluation as ✅ for the container PDP and ❌ for the New Edge PDP was wrong twice over. The row is gone from the parity table and from "Choosing between the two".
Graph-based filtering — which both PDPs do support, via /user-permissions and /authorized_users — is already covered by those rows, and there's now a short note under the table pointing at the Data Filtering guide so readers arriving from there aren't left wondering.
Same reasoning retired the leftover ext-authz mention: v2 doesn't have that either.
|
|
||
| **✅ Supported · 🚧 On the roadmap · ❌ Not supported** | ||
|
|
||
| ### Permission checks |
There was a problem hiding this comment.
I imagine the whole thing as a big table of feature parity not splitted
There was a problem hiding this comment.
Done — one table now, not four split by theme, with the check sub-capabilities (RBAC / ReBAC / multi-tenancy / ABAC) nested under Check.
Merging it did surface a rendering bug: at four columns the table was 732px in a 640px content column with overflow-x: visible, so the New Edge PDP column was silently clipped rather than scrollable. Folding the endpoint into the capability cell drops it to three columns; verified every table on all four pages now fits.
| :::note OpenTelemetry and `/metrics` | ||
| The New Edge PDP does not currently export OpenTelemetry traces, metrics, or logs, does not read `OTEL_*` environment variables, and does not serve a `/metrics` endpoint. OTLP export is on the roadmap — see [Feature compatibility](#feature-compatibility-and-roadmap). | ||
|
|
||
| Observability today is structured stdout logs plus the health endpoints described above. If OTLP export matters for your deployment, [let us know](mailto:support@permit.io). | ||
| ::: | ||
|
|
||
| --- |
There was a problem hiding this comment.
this doesn't feel like the right place to put this on
There was a problem hiding this comment.
Removed. It was duplicating the parity table, which is where OTLP now lives (❌ container PDP, 🚧 New Edge PDP). The observability section just covers what exists today — health endpoints, structured logs, decision logs.
Second round of review feedback from Omer. Terminology. The old PDP is no longer called "Edge PDP" anywhere — that term was confusing and largely unused. It is now the "container PDP" (permitio/pdp-v2), matching the wording already used in cloud-pdp-capabilities.mdx. "New Edge PDP" stays as the name for v3. Splitting. One long page becomes four, wired into the sidebar as a collapsible "New Edge PDP" sub-category with the overview as its landing page: Overview - what it is, why, architecture, choosing between the two How It Works - sync, consistency, latency, cold start, scale, footprint Feature Parity - the comparison table Deployment - requirements, observability, security model Parity table is now a single table rather than four split by theme, with the check sub-capabilities nested under Check. Removes the data-filtering row. Partial evaluation is not a feature of the container PDP either (no compile endpoint in horizon/ or pdp-server/), so scoring it as a v3 gap was wrong in both columns. Graph-based filtering, which both support, is covered by the user-permissions and authorized-users rows and called out in a note below the table. Same reasoning retires the ext-authz mention left in "Choosing between the two". Drops the standalone OpenTelemetry note from the observability section; the parity table is where that belongs and it was duplicated. One rendering fix found while checking the result: the merged table was 732px in a 640px column with overflow-x visible, so the New Edge PDP column was clipped rather than scrollable. Folding the endpoint into the capability cell drops it to three columns. Verified every table on all four pages now fits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟡 Changes recommended
There are documented inconsistencies (notably OpenTelemetry marked as “on the roadmap” vs the PR’s stated “not available / nothing in flight”, plus an incorrect SVG title) that should be corrected to avoid misleading readers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Lite
| | **Decision logs to Permit** | ✅ | ✅ | | ||
| | **Health and readiness endpoints** | ✅ | ✅ | | ||
| | **Prometheus metrics** — `GET /metrics` | ✅ | ❌ | | ||
| | **OpenTelemetry (OTLP traces, metrics, logs)** | ❌ | 🚧 | |
There was a problem hiding this comment.
Good catch on the inconsistency — but the fix belongs on the PR description, not the table.
The description was written from a code investigation that established OTLP is not implemented (no opentelemetry dependency, init_observability never called, no OTEL_* read, no /metrics) and that no branch is in flight. That is evidence about the current tree, not about intent — "nothing in flight" is not the same as "not planned".
A maintainer then reviewed and stated it is on the roadmap, which is the authoritative source for that question. Hence 🚧 for the New Edge PDP and ❌ for the container PDP, which has Prometheus /metrics but no OTLP export.
The PR description was stale against that and has now been updated to say both things: not implemented today, on the roadmap per review. No timeline is stated on the docs pages or in the table.
Copilot flagged that the description says single-binary while the page documents two supervised child processes (OPA and the NATS leaf). It is a real contradiction as written. Rather than swap in "single-container", which is true of the container PDP too and so distinguishes nothing, the framing now leads with what actually is distinctive: an embedded on-disk database, kept in sync over a durable event stream, with no network in the decision path. The precise account of what runs in-process versus as a child stays in the architecture table, which was already accurate. Also drops "fewer moving parts" from the comparison table. Both runtimes run three processes — Python API server + OPAL client + OPA against Rust binary + NATS leaf + OPA — so that claim was false. Replaced with the difference that holds: a compiled runtime with no Python or OPAL layer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟡 Changes recommended
A few doc/accessibility details are likely to mislead readers (notably probe guidance lacking the :7001 port and ambiguous wording in the availability note) and should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
docs/concepts/pdp/new-edge-pdp.mdx:15
- This sentence is ambiguous: “the right choice for the capabilities listed under Feature Parity” can read like the container PDP is the right choice for everything in the table (including capabilities both support). It would be clearer to point to capabilities that are not yet supported by the New Edge PDP.
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Lite
Reviewed every change on this branch against source. Findings: The container PDP's process model was wrong. I had it as "Python API server + OPAL client + OPA" with CPython against v3's compiled binary. In fact permitio/pdp-v2's Dockerfile runs CMD ["/app/pdp"] — the Rust pdp-server — which supervises Python Horizon (the OPAL client) through a watchdog, plus OPA. So v2 is already Rust-fronted on a Python base. Corrected to "Rust API server + Python Horizon (OPAL client) + OPA", and the runtime row now reads "Python runtime: required for the OPAL client / not present", which is the difference that actually holds. That also invalidated "a compiled runtime" as a reason to choose v3, since both are compiled at the front door. Replaced with dropping the Python and OPAL layer, which is real. The diagram labels the change stream WAL and LOCAL_WAL while the prose calls it the change stream, with nothing connecting the two. Added a short note under the diagram. The SVG's accessible title still said "Edge PDP Architecture" — the only remaining bare "Edge PDP" anywhere, and one only screen-reader users would hit. Renamed, and replaced the generic "generated by Archify" description with one that describes the diagram. Deployment requirements said to point probes at /health and /health/ready without a port, two paragraphs above a warning whose entire point is that the port matters. Both now say :7001. Verified and left alone: AuthZen routes are identical on both PDPs (pdp-server/src/api/authzen/mod.rs registers the same /access/v1/* paths), so the parity row is right; nine gating components matches both the source and the diagram's own label; 40s is 10+30 at defaults. All 38 internal links and anchors resolve, no prose is duplicated across the four pages, every table is structurally consistent, and the diagram was checked in-browser in both light and dark. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟢 Approval recommended
The changes are documentation-focused and consistent overall, with only a small wording clarification suggested to avoid probe/health endpoint ambiguity.
Review details
Suppressed comments (1)
docs/concepts/pdp/new-edge-pdp-deployment.mdx:40
- The section starts with "Health is served on the management port (7001)", but a few lines later it also states that port 7000 serves
GET /healthandGET /healthy. Reword the first sentence to make it clear that 7001 is the recommended probe target (and hosts the readiness/detail endpoints), rather than implying health is only available there.
Health is served on the **management port (`7001`)**, which is where all probes belong:
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
|
||
| The container PDP keeps your authorization data inside OPA's in-memory document, and refreshes it by reacting to a notification and then **fetching the changed data back from Permit's API**. That design is proven, but it has three structural ceilings: | ||
|
|
||
| - **Memory scales with your data.** Because the fact graph lives in memory, a large environment needs a large PDP — which is why sharding exists for the biggest data sets. |
There was a problem hiding this comment.
sharding is deprecated and should be removed, please remove it as agents keep referencing it
( remove both from docs and code but at least a ticket at first and remove the reference here )
There was a problem hiding this comment.
Removed — all three references on these pages are gone (this one, the choose-between-the-two row, and a sentence in the resource-footprint section). The memory argument now rests on the OPA 20x citation instead of on sharding being the container PDP's answer.
On the wider cleanup: seven other files in this repo still reference sharding —
how-to/deploy/overview.mdxhow-to/deploy/on-prem/management.mdxhow-to/deploy/on-prem/reference.mdxhow-to/deploy/on-prem/installation.mdxhow-to/manage-data/loading-data.mdxconcepts/pdp/overview.mdx(has a "Sharded Edge PDPs" section)concepts/pdp/configuration.mdx
That is outside this PR's scope, but it is the set a cleanup ticket would need to cover — concepts/pdp/overview.mdx especially, since a whole section there presents sharding as a recommended deployment model.
|
|
||
| | Choose the **container PDP** when you need | Choose the **New Edge PDP** when you need | | ||
| | --- | --- | | ||
| | ABAC, condition sets, user or resource sets | The largest data sets, without sharding | |
There was a problem hiding this comment.
sharding is deprecated and should be removed, please remove it as agents keep referencing it
( remove both from docs and code but at least a ticket at first and remove the reference here )
There was a problem hiding this comment.
Removed here too — that row now reads "Large data sets without a large memory footprint", which is the actual benefit rather than a comparison against a deprecated feature.
|
|
||
| The container PDP keeps your authorization data inside OPA's in-memory document, and refreshes it by reacting to a notification and then **fetching the changed data back from Permit's API**. That design is proven, but it has three structural ceilings: | ||
|
|
||
| - **Memory scales with your data.** Because the fact graph lives in memory, a large environment needs a large PDP — which is why sharding exists for the biggest data sets. |
There was a problem hiding this comment.
I'd say referencing this sentence from OPA docs is a good idea here
Memory usage scales with the size of the policy (i.e., Rego) and data (e.g., JSON) that you load into OPA. Raw JSON data loaded into OPA uses approximately 20x more memory compared to the same data stored in a compact, serialized format (e.g., on disk)
There was a problem hiding this comment.
Added as a blockquote with attribution and a link to OPA: Policy Performance. Much better than my hand-waving — it turns "memory scales with your data" into a number from an authoritative source, and 20x is a striking one.
The resource-footprint table on the How It Works page now carries the same citation, replacing the sentence that pointed at sharding.
|
|
||
| --- | ||
|
|
||
| ## Why a new runtime |
There was a problem hiding this comment.
add here something about that opa data system is a single json document that isn't and can't be optimized for actual graph traversal or complex granular permissions query requirements
There was a problem hiding this comment.
Added, and it reframed the whole section — this is the root cause rather than a symptom, so it now leads.
"Why a new runtime" opens with OPA's data store being a single in-memory JSON document, then draws the two consequences: no indexes and no concept of edges, so it cannot be optimized for graph traversal or for the relationship-heavy queries ReBAC and derived roles need; and the memory cost.
It also connects forward — the New Edge PDP moves those queries out of the JSON document into a store built for traversal, which OPA reaches over the query loopback while it evaluates policy. The Architecture page makes the same point against the request path.
|
|
||
| --- | ||
|
|
||
| ## Architecture |
There was a problem hiding this comment.
i'd say architecture needs its own subpage explaining the architecture, which part is responsible of what and stuff like that while the "root" page stays thin about the general idea, why it was established, what it allows and stuff like that
There was a problem hiding this comment.
Done — Architecture is now its own subpage. It carries the diagram, the per-component responsibilities, a port table, and the request path, plus a new section on how the read path and the sync path stay separate.
The root page is now just the general idea, why it exists, what it gives you, and the choose-between-the-two table — down from ~380 lines originally to 87.
Third round of review feedback from Omer. Architecture moves to its own subpage so the root page can stay thin. The root is now the general idea, why it exists and what it gives you; Architecture carries the diagram, the per-component responsibilities, the port table and the request path, plus a new section on how the read path and the sync path are kept separate. Root page is down to 87 lines. "Why a new runtime" now names the actual root cause rather than the symptoms. OPA's data store is a single in-memory JSON document, which sets two ceilings: it has no indexes and no concept of edges, so it cannot be optimized for graph traversal or for the relationship-heavy queries ReBAC and derived roles need; and it is expensive, which OPA's own documentation quantifies at roughly 20x the memory of the same data in a compact on-disk form. Both points are quoted and linked to the OPA docs rather than asserted. The third ceiling, the notify-then-fetch round trip, follows. Sharding is gone from these pages — all three references. Note that seven other files still mention it (how-to/deploy/overview, on-prem management, reference and installation, manage-data/loading-data, concepts/pdp/overview which has a "Sharded Edge PDPs" section, and concepts/pdp/configuration); those are outside this PR and need their own ticket. The resource-footprint comparison now carries the same 20x citation instead of pointing at sharding as the container PDP's answer for large data sets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟢 Approval recommended
The changes are additive documentation-only updates with consistent sidebar wiring and no verified correctness or build-breaking issues found in the modified files.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
Omer asked for the configuration page back, marked beta and open to change. Restored from 054aaec^ and brought up to date with the current structure. The beta framing is the point, and it resolves the objection that got the page removed. It now opens with a caution saying the surface is not stable, that variables may be renamed, replaced or removed before GA, that it should be treated as a working reference rather than a contract, and that readers should pin an image tag and re-read on upgrade — plus an invitation to tell us if they depend on a specific variable so we can flag it before it moves. Updated from the version that was removed: "container PDP" throughout instead of "existing Edge PDP", sidebar position after Deployment, and deep links into the pages that did not exist when it was written — the port table on Architecture, health-and-readiness and deployment-requirements on Deployment, resource-footprint on How It Works. The storage-engine tuning advice on How It Works now points back here instead of at support, and every page carries a Configuration entry in its related docs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W
There was a problem hiding this comment.
🟡 Changes recommended
The PR description’s stated scope (page set and configuration-documentation intent) conflicts with the docs and sidebar content added in this PR, and should be aligned before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 1
- Review effort level: Lite
| "concepts/pdp/new-edge-pdp-feature-parity", | ||
| "concepts/pdp/new-edge-pdp-deployment", | ||
| "concepts/pdp/new-edge-pdp-configuration", | ||
| ], |
Adds documentation for the new PDP runtime (
permitio/pdp-v3), documented as the New Edge PDP — an additional deployment option. The existingpermitio/pdp-v2is referred to throughout as the container PDP and is framed as fully supported; the parity table reads as "what the new one covers so far", never as removals.Four pages, wired into
sidebars.jsas a collapsible New Edge PDP sub-category under The Policy Decision Point (PDP):Accuracy
Every endpoint, port, and behaviour was verified against the
cloud-pdpsource rather than that repo's README or staging runbook, both of which are stale in several places.Two errors caught during review, both wrong in both columns:
horizon/orpdp-server/— neither runtime has it. Row removed; graph-based filtering (which both support) is covered by the user-permissions and authorized-users rows.Known gaps, documented as such
OpenTelemetry
Not implemented in
edge-pdptoday — it never callsinit_observability, has noopentelemetrydependency, reads noOTEL_*variables, and serves no/metrics. Both in-repo OTEL branches predate the crate.Per review, it is on the roadmap, so the parity table marks it 🚧 for the New Edge PDP and ❌ for the container PDP, which has Prometheus
/metricsbut no OTLP export. No timeline is stated anywhere.Configuration
Deliberately not documented yet — the environment variable surface is still settling, and publishing a reference now would ship something known to go stale. The operational requirements that readers act on (persistent storage, ≥40s
terminationGracePeriodSeconds, probe targets, memory sizing, ports) live on the Deployment page without pinning variable names.Follow-up found along the way (not fixed here)
docs/concepts/pdp/cloud-pdp-capabilities.mdx:59-63lists the AuthZen paths as/v1/access/evaluation,/v1/subjects,/v1/resources,/v1/actions. The actual routes are/access/v1/evaluation,/access/v1/evaluations,/access/v1/search/{subject,resource,action}. That same page's rate-limit table at lines 88-93 already uses the correct form, so it contradicts itself.Verification
npm run redirect-lint— cleannpx docusaurus build— succeeds, no broken links; cross-page anchors confirmed to resolve🤖 Generated with Claude Code
https://claude.ai/code/session_011ghyfrjy3SGKPV8ruaXE8W