Update validation/publication to use snapshots - #3155
Conversation
…n, I still need to review and understand this further..
…ide of the test harness and other basic documentation related things. I did checked into the widening of security access requirements through the use of authz I was able to confirm that it is wider than before, because before the lookup was done based on prefixes.
jgraettinger
left a comment
jgraettinger
left a comment
There was a problem hiding this comment.
Directionally this looks right, and I don't see a problem with the AuthZ switchover. Some comments below.
|
Remaining uses of internal.user_roles
These will be fixed in later PRs |
jgraettinger
left a comment
jgraettinger
left a comment
There was a problem hiding this comment.
Progress! Some comments below. In general it needs to go further in terms of pushing down the Snapshot and having it be the full basis of AuthZ checks. Directionally, we should be entirely removing authz concerns from DB queries. We may also be able to remove some cruft like the spec capabilities that attach to fetched live spec rows (check it elsewhere, using the snapshot -- spec capabilities had run alongside the spec only because that was formerly the sole channel for fetching them out, but we have snapshots now).
… discovers.rs that spider webbed out to other files.
Rather than blindly re-polling every few seconds after an authorization denial under a stale Snapshot, publications and discovers now persist the instant an authoritative Snapshot must postdate (awaiting_snapshot_after, in internal.tasks, so whichever agent instance dequeues the next poll applies the same criterion) and defer re-polls - without loading, building, or connector work - until the local Snapshot postdates it. Deferred polls wake on a constant 20s interval (Snapshot::STALE_RETRY_WAKE == MIN_REFRESH_INTERVAL), and deferral is abandoned once MAX_REFRESH_INTERVAL plus two wake cycles has elapsed, past which refreshes are failing and the attempt proceeds rather than gating the task forever. Publisher no longer holds a Snapshot watch: DraftPublication carries &Snapshot, pinned once per executor poll (and supplied by controllers, data-plane creation, and L2 reporting from their own watches), so the deferral decision and every authorization decision observe one view. Also hoist the three-way authorization classifier from publications::specs onto Snapshot::resolve_authorization, returning an Authorization enum (Authorized / Denied / Stale) with an ok_or_stale adapter. The same policy now serves resolve_live_specs, get_live_specs, get_connected_live_specs, and the discover data-plane precheck, which each previously hand-rolled it.
# Conflicts: # crates/agent/src/discovers.rs # crates/agent/src/publications.rs
Control-Plane Upgrade Test:
|
| Test | master agent | branch agent (post-upgrade) |
|---|---|---|
Positive publish (alice → acme/) |
works | ✅ works, ~2s, no defer |
Negative publish (alice → zeta/) |
denied in ~5s | ✅ denied in ~29s after one defer cycle |
Positive discover (alice, source-hello-world) |
works | ✅ works, ~3.4s |
| Negative discover (carol, no data-plane read) | noDataPlane in ~3s |
✅ notAuthorized in ~24s |
| Race: grant committed 36 ms before publish | (not exercised) | ✅ defers once, then Success in ~27s |
Log evidence (upgraded agent)
Negative publication — provisional denial, deferral, then authoritative denial:
11:38:21 ERROR control_plane_api::publications: error=authorization for zeta/tests/intrusion-test
was evaluated against a control-plane snapshot that is not authoritative for this
operation; please retry the operation
11:38:21 INFO agent::publications: publication authorization snapshot is stale; rescheduling
11:38:45 INFO agent::publications: publication finished status=JobStatus { type: BuildFailed }
Negative discover — stale defer, then the new authoritative NotAuthorized status:
11:39:32 WARN agent::discovers: data-plane read denied under a stale snapshot
11:39:32 INFO agent::discovers: control-plane snapshot is stale; rescheduling discover after refresh
11:39:53 WARN agent::discovers: user is not authorized to read data plane
11:39:53 INFO agent::discovers: finished status=NotAuthorized
Race — grant at 11:40:14.004, publish submitted at 11:40:14.040:
11:40:18 INFO agent::publications: publication authorization snapshot is stale; rescheduling
11:40:40 INFO agent::publications: publication finished status=JobStatus { type: Success }
Findings
- Deploy compatibility holds. Publications and discovers queued/persisted by
master round-trip cleanly through the branch agent (theOption<State>-as-null
poll-state design), and grants created on master authorize immediately
post-upgrade with no defer. - Behavioral deltas are the designed ones.
- Denials now cost one ~20s defer cycle (
STALE_RETRY_WAKE = MIN_REFRESH_INTERVAL)
before becoming authoritative; master denied instantly from direct DB reads. - The discover status for an unauthorized data-plane read is now the honest
notAuthorized, where master conflated it withnoDataPlane. - A grant committed moments before an operation is picked up via defer-and-retry
instead of failing spuriously — the core scenario this branch fixes.
- Denials now cost one ~20s defer cycle (
State left in place (for inspection)
- Checkout on
bmb/2781-publications; stack running with the branch agent. - bob holds the race-test
acme/admin grant
(user_grants.detail = 'claude race test round 2'). carolco/ → ops/dp/public/ readremains deleted fromrole_grants.
|
More claude orchestration tests. Orchestration E2E Tests: Stale-Snapshot Deferral FeatureDate: 2026-07-31 Environment note (incident during setup)Between the previous session and this one, the dev catalog had been wiped: running Test 1 — Rollback with non-null defer stateClaim under test: a publication that the branch agent has deferred — with a What was done
Outcome — PASSED
Timing
Test 2 — Revocation raceClaim under test: the deferral mechanism only protects denials; an What was done Leg 1 — revoked in DB, still granted in snapshot:
Leg 2 — revocation reflected after refresh: Outcome — PASSED (both legs)
Timing
Test 3 — Restart mid-deferClaim under test: the defer state is durable, not in-memory. The code comments What was done
Outcome — PASSED
Timing
Summary
All three previously-untested claims of the feature — rollback decodability, System state left in place: merged branch checked out and its agent running; |
Rollback Test: In-Flight Discovery & Publication Across a Release RevertDate: 2026-07-31 Claim under testIf a release running the stale-snapshot deferral feature must be rolled back to
P1/D1 were arranged to succeed after rollback (grants committed just before What was doneArrangement — bob held no
Outcome — PASSEDAll four operations returned successfully within 3.3 seconds of the master
Timing
Findings
System state left in place: merged branch checked out, its agent running; |
jshearer
left a comment
jshearer
left a comment
There was a problem hiding this comment.
Alright! I took one more pass over this from a DRY and comment-correctness and have the following feedback, then I think we're good to go to get this in early next week. Thanks a lot for doing the validation above, that's great 👍
…_connected_live_specs and updated tests and other impacted parts of the code as well.'
# Conflicts: # crates/control-plane-api/src/evolutions/mod.rs
GregorShear
left a comment
GregorShear
left a comment
There was a problem hiding this comment.
Sorry for the drive by here, but i'd like to see us adopting the new authz model rather than adding more instances of legacy read/write/admin that we'll have to replace soon anyway
| for name in candidate_data_plane_names { | ||
| if !verify_user_authz | ||
| || snapshot | ||
| .user_authorization(user_id, name, models::Capability::Read, started) |
There was a problem hiding this comment.
@jshearer any suggestions for which capability bit to require in place of Read here? Do we need to make a new one?
There was a problem hiding this comment.
Yes, I think we should make a new one named DataPlaneDeploy (or something like that) and roll that into the existing Viewer bundle so the behavior of legacy read capability conferring deployment permission continues to work. We should also add it to the create_data_plane codepath like we did with ViewDataPlanePrivateNetworking.
That being said, I dont think this needs to land in this PR necessarily. do you?
There was a problem hiding this comment.
I would like to move the change to the next PR if possible.
There was a problem hiding this comment.
Or even better one after the next one.
…ers on staleness The discovers executor moves its data-plane authorization onto the Snapshot and gains the same defer-on-stale behavior as publications, completing the removal of internal.user_roles() from the discover path: - The inline SQL data-plane query is replaced by Snapshot::user_authorization plus data_plane_by_catalog_name, distinguishing the three cases the SQL conflated: an authoritative denial is the new terminal JobStatus::NotAuthorized; a denial under a Snapshot predating the discover requests an early refresh and retries; and a plane genuinely absent from an authoritative Snapshot remains NoDataPlane, with a refresh-and-recheck grace for planes registered after the Snapshot was taken. - started_at anchors staleness to the queued discover row, and AuthorizationSnapshotStale from live-spec reads reschedules via DiscoverState::awaiting_snapshot_after instead of reporting a spurious failure. DiscoverOutcome becomes Resolved / RetryStale. - Harness: add_data_plane, queue_discover, discover_job_status, and SnapshotRefresher for refreshing from 'static connector fixtures. After this change the stacked tree is identical to #3155. Split 4 of 4 from #3155.
|
Note that this branch was migrated to the following PR #3341 |
4 participants
Ticket 2781
Closes #2781
Description:
Motivation
Publications, discovers, and evolutions authorized each operation by running recursive
internal.user_roles()queries inside Postgres. The agent already maintains an in-memory authorizationSnapshot(user grants, role grants, data planes) used for task and journal authorization. This PR moves catalog authorization for those operations onto that same snapshot, replacing the recursive SQL with exact in-memory checks.Current design
publications::specs::resolve_live_specs, discovers, and evolutions are made bytables::UserGrant::is_authorizedandtables::RoleGrant::is_authorized, evaluated against aSnapshot.fetch_live_specsis now a plain fetch-by-catalog-name; the user filtering that previously happened in SQL is applied in memory afterwards (likewiselive_specs::get_live_specs/get_connected_live_specs).&Snapshotthrough every phase of an attempt: publication initialize → resolve → build, and an entire discover including across the connector RPC.Why one Snapshot is pinned per operation
Every check within an attempt sees one consistent point-in-time view, so there are no torn decisions where one phase observes a grant another phase doesn't. It also makes the freshness contract well-defined: staleness is a property of the snapshot relative to the operation, not of whichever snapshot each phase happened to grab. Asserted by
test_publication_uses_one_snapshot_across_phasesandtest_discover_uses_one_snapshot_across_connector_rpc.Freshness contract
A snapshot-based denial is authoritative only if the snapshot was taken after the operation was queued; otherwise the denial may just be a grant the snapshot hasn't observed yet.
updated_at, threaded asDraftPublication::started_at. It is durable across retry attempts, which is what lets retries converge (a per-attemptnow()could never be overtaken by a snapshot).Snapshot::taken_after(anchor)is the single definition of "authoritative for this instant", shared withenvelope.rsand task authorization, and allows 250ms ofTEMPORAL_SKEW.authz_snapshot_staleerror: the snapshot'srevoketoken is cancelled to request an early refresh, the row staysqueued, and the executor sleeps and re-polls (5s backoff) until a snapshot postdating the request is observed.started_at: Noneand fall back to anchoring each denied spec on its ownlast_pub_idtimestamp. Named data planes have no equivalent fallback, so their denials are terminal for those callers.Authorization policies evaluated
Within
resolve_live_specs:adminto the catalog name.read.reads_fromsource and write-authorized to eachwrites_totarget (RoleGrant). These are always enforced, even when user checks are skipped.readto the named/default data plane and to storage-mapping data planes.verify_user_authz: false(pre-authorized system publications: controllers, data-plane creation, L2 reporting) skips only the user-level checks above.Test matrix
snapshot.rstaken_afterskew boundary semanticspublications/specs.rs(sqlx)reads_from/writes_togrants, read-on-referenced, authorized draft resolves cleanly, terminal vs. stale denial with and withoutstarted_at, spec-relative anchor fallback, data-plane and storage-mapping data-plane freshness, attenuated grants not visible, spec-to-spec staleness applies even withverify_user_authz: falselive_specs(sqlx)user_publications.rs(integration)user_discovers.rs(integration)authz_specs.sql,attenuated_grants.sqlEventual consistency and deployment
MIN_REFRESH_INTERVAL(20s) andMAX_REFRESH_INTERVAL(5m); stale denials request an early refresh.internal.user_roles()call sites (directives, evolutions' own DB queries, and theops/admin pre-checks in create-data-plane / update-L2-reporting) are unchanged and can migrate separately.Workflow steps:
No user-facing workflow changes. Publications, discovers, and evolutions behave as before, except that authorization denials evaluated against a stale snapshot now retry instead of failing.
Documentation links affected:
#2782 provides additional content.
Notes for reviewers:
The invariants to verify are: (1) exactly one snapshot is resolved per operation attempt and threaded through all phases; (2) the staleness anchor is durable across retries (
publications.updated_at), so stale denials converge to either success or an authoritative denial; (3) spec-to-specRoleGrantchecks are never skipped, regardless ofverify_user_authz; (4) stale denials are surfaced as retryable errors and never reported to users as authorization failures.