Repository navigation
feat(breg): add read-only external views over Adapter projections - #2634
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b12fb60c29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a4459da23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Baseline of the external record projections candidate: BReg serves read-only views over records an Adapter projects from an external source, with grants checked on every read, sealed page cursors and a retained projection store. Review fixes follow in separate commits. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
A cached external value now lives for the authored cacheTtlSeconds, which a deployment binding may only shorten, and is purged when it expires. A separate retention window, refresh interval and purge deadline added configuration without adding protection, so they are gone from the binding, the store and the source facts. The deployment allowedSelectors list is also removed. Local row authority already decides which roots may be acquired. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Scope fingerprints now cover only the reviewed source contract, its operation and the allowed fields, so a renewed grant cutoff, a CA change or a connection edit no longer discards the cache. Each projection binds on its own: a contract mismatch rotates to a fresh epoch, and a projection that still cannot bind disables only its views with an unavailable Problem that names it. Startup and doctor report unavailable projections with the reason. Deployment configuration is the only acquisition cutoff: withdrawal purges data and a binding still configured binds again at the next start. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The bregctl generate target list gained external-source-facts with external views; the published reference test still expected the earlier list. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Operators maintained a projection cache scope through a psql script that took digests, epochs and activation IDs copied by hand. bregctl now reads the runtime configuration, computes the binding and grant digests and the effective cache lifetime as startup does, and acts on one indexed projection under the configured migration role. Withdraw and rotate take the registry lock, check the active package, activation and managed catalog in the changing transaction, and are audited under breg-external-projection-audit/v1. Inspect reads the scope and fixed counters only. The scope script is removed; the recovery drill and the operator guide use the bregctl commands. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Incomplete indexed coverage now returns an external_view.incomplete Problem instead of a bare status body. Scope lock and database failures on the read path answer source.unavailable rather than incomplete coverage or an invalid cursor; only a withdrawn or rebound projection keeps the detail fallback and cursor refusal. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
An unavailable, incomplete or unbindable refresh for one key no longer removes the previous snapshot or bumps the scope generation. The snapshot stays readable until its own expiry; only explicit denial or no-store removes it, so a transient source failure no longer turns covered indexed reads into incomplete coverage. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
… refreshes The scope generation now moves only on rotation, withdrawal and contract reset, so a reader's continuation survives concurrent publication. A label sweep confirms that the exact parent snapshots supplying its selectors are still current before each source call, instead of stopping whenever any parent root refreshed. Publication still rechecks current membership in the same statement. A database failure while rechecking the parent is reported as unavailable instead of abandoning the page. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The test-only helper publishes without the root eligibility statement that runtime publication always wraps around the store function. Its name now says so, so a reader cannot mistake it for the guarded path. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
A refresh receipt now stays live for the time left before the page deadline plus a ten second margin, instead of a fixed two minutes. A page that runs longer than the old lease but inside its own deadline still publishes, and a short page no longer holds receipts live longer than it can use them. The store refuses leases outside one second to one hour. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
A refresh page published every key in one transaction, holding the projection scope row from its first publication to commit. Indexed reads share that row, so they waited for the whole page and fell into their lock timeout. Each outcome now commits on its own, after rechecking the projection epoch and, for label sweeps, the parent generation. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
An HTTP continuation under another principal or another row boundary claim answers query.cursor_invalid, and the original caller continues. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
A date's text output follows the session DateStyle, so a cursor issued under one style failed to continue under another. The cursor now carries the ISO 8601 form, which the continuation parses under any style. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The cursor binding used the operator's Rust Debug form, which changes with a rename. It now uses the contract names equals and in, as query bindings do. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
A dense eligible scope already pages in primary-key order without a sort, and a selective row boundary pages faster with a bounded sort over its few rows than with a forced primary-key walk. The page query no longer turns sorting off; a test pins the dense-scope plan. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…ding A source reply marked no-store refuses an indexed acquisition through the bound source, and the refresh records it as a retention refusal, which removes the previous snapshot. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
No caller ever built the no-store variant; the Adapter refuses a no-store reply before a snapshot exists. The store keeps its check that retention covers the source expiry. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Operators could not tell why external projection data went stale or stayed stored: a failed purge pass logged no cause, refresh outcomes were not logged, and a failed live acquisition was discarded. Each purge pass now logs the rows it removed per projection and reason (expired, replaced, withdrawn), a failed pass logs its failure category, and a pass that reaches its batch bound warns. Each refresh page logs its published count and its failed keys per reason, a refresh that fails as a whole logs its reason, and a failed live acquisition logs its reason instead of being dropped. No line carries a source key, selector, value or secret. A startup check also reports, for each projection a server would serve, the source grant expiry and the observation time of the newest snapshot its active epoch holds. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
bregctl doctor now adds an external-projection.status advisory for each projection a server would serve. It names when the source grant expires and, for an indexed projection, when the newest snapshot it holds was observed, so an operator sees a lapsing grant or a stalled refresh sweep before readers do. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…ntions lint Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…e them on write Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…feature Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…efresh Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…o-store win over failures A 401 answered by a successful OAuth credential refresh no longer poisons the acquisition, and a no-store response now yields NoStore even when the status or route failed. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…in the load-test runners Resolves the CodeQL uninitialized-local and unused-import alerts without changing what the runners do. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…tion on credential failures Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…pt snapshot Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…tion builds its record Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…of failing the batch Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…accepts Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…isition Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The corpus gate requires every registered authoring code to have a member; adapter.pack.projection-invalid had none, so the Adapter contract script failed once the conventions lint passed. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The request audit now records a client reference for every request (BREG-SEC-203). External view reads and their test worker take the same context, so the journal tells OAuth clients apart on these reads too. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…audit context change Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
3a4459d to
322a624
Compare
…up to the cursor bound Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…fields are present Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Pull Request
Summary
Adds read-only external views to BReg. A registry can show and filter
selected facts another institution keeps, through an Adapter projection,
while it stores only its own facts and the identifiers that link them.
Owning area: BReg (
registry-breg,registry-bregctl), with the Adapterprojection contract it reads through.
Rebased onto #2630. #2630 added BREG-SEC-203 (the journal tells OAuth
clients apart), so this PR's invariants move to BREG-SEC-204 to 207 with
their negative tests, and external view reads take #2630's per-client
request audit context.
projections (live or indexed, one or many) and views whose links read
through them. Indexed views list, filter, sort and count over a kept
copy; live views read on demand.
binding and grant fingerprints cover the pack contract, the operation
and the allowed fields, and a changed cache lifetime starts a fresh
epoch. Renewing a grant cutoff, replacing a CA bundle, moving an
endpoint or changing acquisition limits keeps the cache. Each projection
is checked on its own, so a mismatch takes only its views offline.
restarting stops acquisition; its views answer
source.unavailable.bregctl external-projection withdrawpurges retained values but doesnot stop acquisition. Backups exclude projection data, so a restore
starts with no source values, as on a fresh install.
A successful source response marked
no-storenever enters the storeand removes the previous snapshot. A
no-storeon an error, a redirector the challenge that starts a credential refresh changes nothing, so a
refresh that fails keeps the previous snapshot. A credential failure
during an acquisition fails the projection, even when a route step
catches it.
bregctl external-projection inspect|withdraw|rotate,doctor checks with the grant horizon, and monitoring codes.
value, and is never sent to the source. A malformed key (over 512 bytes,
surrounding white space, a control character) is never sent either: that
record alone reads as having no outside value, the refresh and live reads
count and log it as
invalid-key, and lists, filters, sorts and countskeep serving the other records. Writing a malformed key to a field an
external view links from is refused as
request.invalidon create,patch, action, proposal, batch and import writes; proposals and action
input are refused when submitted, before anything is stored. The same
value in a field no view links from is accepted.
and indexed paths. Integers keep their exact digits; other numbers are
spelled without an exponent, so one very small or very large value no
longer fails its batch.
accepts. A page whose last row carries a sort value too long to resume
from answers
source.unavailablewith nonextCursor, after the earlierpages were served.
and indexed reads look up each root's snapshot by primary key. Scale and
recovery runners live under
products/breg/loadtest/external-views/.Evidence
Run locally against a disposable PostgreSQL database.
On the branch head, after the rebase onto #2630:
cargo fmt --all -- --checkcargo clippy -p registry-breg -p registry-bregctl --all-targets -- -D warnings(one run, as CI lints affected packages together)cargo test -p registry-breg --lib --features postgres-test,tooling,schemacargo test -p registry-breg --features postgres-test --test postgres_mutation; the same with--test external_viewsproducts/breg/scripts/check-contracts.sh;products/adapter/scripts/check-contracts.shadapter.pack.projection-invalidproducts/identifiers/scripts/check.shpython3 -B -m unittest test_runners test_scaleinproducts/breg/loadtest/external-views/cargo clippy -p registry-breg --all-targets --features postgres-testnow stops on a
type_complexitylint intests/postgres_migration.rsthat is already on main; CI does not lint that feature set.
On an earlier head of this branch, before the rebase; CI runs them on this
head:
cargo clippy -p registry-adapter-engine --all-targets -- -D warnings;cargo test -p registry-adapter-engineproducts/breg/scripts/test-postgres.shcargo clippy -p registry-bregctl --all-targets -- -D warnings;cargo test -p registry-bregctl --test cli;cargo test -p registry-bregctl --libTMPDIRon macOS for one socket path test)npm run checkindocs/siteTests covering the review fixes:
a_root_without_a_link_key_is_covered_and_reads_absentandthe invalid-key refresh and live-read tests in
external_runtime.rs;invalid_label_selectors_are_unavailable_without_failing_the_rest_of_the_label_batch;an_unlinked_root_reads_as_a_complete_empty_related_collection;a_malformed_link_key_is_refused_when_a_patch_proposal_is_prepared,a_malformed_link_key_is_refused_when_a_create_proposal_is_prepared,a_malformed_link_key_is_refused_when_an_action_builds_its_effect_document,and
the_write_path_refuses_a_malformed_link_key_on_the_linked_field_only(red when the write-time check is removed);
exponent_range_decimals_are_served_as_plain_strings;a_cursor_is_issued_only_for_a_sort_value_it_will_accept;a_caught_failure_of_the_initial_authorization_fails_the_acquisition,a_replayed_response_that_is_not_data_cannot_forbid_retention,a_202_marked_no_store_keeps_the_previous_snapshot(each red when theguarded line is reverted);
external_label_acquisition.rsandan_indexed_read_looks_up_each_root_snapshot_by_its_primary_key;external_projection_failures_each_carry_one_code,a_refused_runtime_configuration_is_reported_as_status_reports_it,external_projection_operations_report_database_prerequisites_with_the_shared_codes,a_moved_activation_is_reported_as_not_ready.The planner-shape tests depend on PostgreSQL planner behaviour (default
selectivity, collation match); comments in the tests say so. They forbid
a plain Sort and bound rows read, and allow an Incremental Sort.
Notes
view keeps reading as no outside value until it is corrected; the
refresh report and logs give the count. A proposal stored before the
link carries its key to Apply, where the write is refused as
request.invalid. Drafts validate inputs, not entity fields, so a draftaccepts a malformed key until it is submitted. A mutation in an action
hook's answer is refused as
hook.proposal.mutation_refused, andhistory-migration SQL steps write without the check; reads guard those
rows. An invalid label key still leaves only that related row without
its label.
with more than about 15 significant digits is rounded before BReg serves
it. BReg does not accept decimal strings from a source.
$skiptokenlength: a cursor payload may reach 8 KiB, which is about11 KiB once encrypted and base64url-encoded in the URL. A proxy with an
8 KiB request-line limit refuses such a next page. Only sorts over long
text values reach that size; a deployment that sorts on them should
raise the proxy limit.
no-storeon an unaccepted 2xx: egress honoursno-storeon any 2xx,including a status the pack does not accept, so such a response removes
the previous snapshot. This fails closed (data the source asked not to
keep is dropped), and is left as it is.
rust/cleartext-storage-databasealerts point into#[cfg(test)]PostgreSQL test modules (external_view_query.rs,external_projection.rs,external_label_acquisition.rs); they aretest fixtures, not runtime storage, and are to be dismissed as used in
tests.
adapter.pack.projection-invalidcode had no bad-authoring corpus member, so the Adapter contracts failed
at their last step. This push adds the member (an
exactprojectionwhose selector and record key name different schemes) and its README
row.
update in Support Studio review, action dates, and workflow timing #2630.
Cargo.lock, BReg API and model,registry-bregctl/src/lib.rs, the CLI docs catalogue, the securitymatrix and traceability files, the authoring schema). Whichever merges
second rebases.
bregctl external-projectionreports a refusedruntime file with the same codes and exit status (1) as
bregctl status.external-projection.unavailableis now the doctor warning only.--exclude-table-dataoptions for packages with indexed projections; restore assumes them.
test(breg): the source successor test and frozen_rhai_package leak a container, a volume and a temp dir every run #2632 (test leaks), evidencectl dev: reading a state file refuses a valid file renamed over between lookup and open #2633 (
evidencectlstate read false refusal).products/breg/scripts/test-postgres.shdoesnot run
registry-breg --lib, so the external projection maintenancePostgreSQL tests run only in CI, and they need the
toolingfeature(with
postgres-testalone the filter matches nothing). Clippy withpostgres-test,tooling,schema --all-targetsfails on a type-complexitylint in
crates/registry-breg/tests/postgres_migration.rs, code alreadyon main.
DCO
Signed-off-bytrailer.