From 937fd6382d07a8446a9807b6aabfb8e42154b0c7 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Tue, 26 May 2026 17:49:38 +0200 Subject: [PATCH] mr-668: remove POST /graphs and CLI graphs create (defer runtime graph mgmt) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POST /graphs runtime-create endpoint shipped in PR 7/10 has three unresolved high-severity bugs: - flock-on-renamed-inode race: the YAML flock is taken on omnigraph.yaml itself, then a temp file is renamed over it. Cross-process writers end up locking different inodes — both believing they hold exclusive access. - duplicate-check outside the file lock: precheck runs against the in-memory registry only; the locked closure does config.graphs.insert(...) unconditionally. Concurrent same-id POSTs can persist the loser in YAML while the in-memory registry keeps the winner — they disagree after restart. - best_effort_cleanup_init_artifacts deletes _schema.pg / _schema.ir.json / __schema_state.json on any init failure. An accidental re-init against an existing graph's URI destroys its schema; subsequent open() fails at read_text(_schema.pg). The correct fix is a Lance-style cluster catalog (reserve → init → publish with recovery sidecars), parallel to the engine's existing __manifest discipline. That work is out of scope for v0.7.0. For now, disable runtime add/remove from the network and CLI surface. Operators add graphs by editing omnigraph.yaml and restarting. The GET /graphs read-only enumeration stays. Removed: - POST /graphs handler + router fragment + utoipa registration - 13 post_graphs_* server tests + 3 composite POST tests + multi_mode_app_with_real_config / post_graph helpers - CLI omnigraph graphs create subcommand + its handler + cli.rs tests - system_remote.rs combined list+create test trimmed to list-only - YAML rewrite infra: rewrite_atomic[_with_modify], RewriteAtomicError, staging_path, hash_config_file, AppState::config_hash field + threading through new_multi and open_multi_graph_state - fs2 dependency (verified absent from cargo tree) - sha2/fs2 imports in config.rs (only the rewrite path used them) - Cedar PolicyAction::GraphCreate variant + "graph_create" match arms + action def in Cedar schema + graph_create_action_authorizes_against_server_resource test - GraphCreateRequest / GraphCreateResponse / GraphSchemaSpec / GraphPolicySpec API types (only the POST handler / CLI imported them) Kept: - GET /graphs (read-only enumeration) and graph_list Cedar action - omnigraph graphs list CLI subcommand - All multi-graph startup, mode inference, cluster routes, per-graph + server-level Cedar policies - server_settings_drive_multi_graph_startup_end_to_end (the test that covers operator-authored YAML + restart — the path that survives) - best_effort_cleanup_init_artifacts and the three init failpoints (still reachable from CLI `omnigraph init`; preflight fix deferred as a follow-up) - GraphRegistry::insert and its concurrency tests — production callers gone, but the method is the natural seam for the future cluster-catalog work Also fixed (transcript issue 4): - ALWAYS_FLAT_PATHS now includes /graphs so multi-mode OpenAPI advertises the management route correctly (was previously rewritten to /graphs/{graph_id}/graphs) - multi_mode_openapi_keeps_healthz_flat → renamed to multi_mode_openapi_keeps_management_paths_flat, asserts both /healthz and /graphs stay flat - multi_mode_openapi_prefixes_operation_ids_with_cluster skips /graphs in addition to /healthz Doc fixes: - docs/user/cli.md: graphs list example was --target http://..., but --target is a config-graph-name lookup; corrected to --uri. Removed the graphs create example. - docs/user/server.md: dropped POST /graphs row, "omnigraph.yaml ownership", and "POST /graphs body shape" sections. Added a paragraph stating runtime add/remove is not exposed in v0.7.0. - docs/user/policy.md: dropped graph_create action; reworded the "Configuration" line to clarify that server-scoped rules (graph_list) take neither branch_scope nor target_branch_scope. - docs/releases/v0.7.0.md: rewrote release narrative — multi-graph mode ships; runtime add/remove deferred. - AGENTS.md: HTTP server bullet and capability matrix row updated to reflect read-only GET /graphs and the operator-edit workflow. - openapi.json regenerated; /graphs has only .get, no .post. Diff: 17 files, +123 −1525 LOC. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 4 +- Cargo.lock | 33 -- crates/omnigraph-cli/src/main.rs | 100 +--- crates/omnigraph-cli/tests/cli.rs | 69 +-- crates/omnigraph-cli/tests/system_remote.rs | 59 +- crates/omnigraph-policy/src/lib.rs | 67 +-- crates/omnigraph-server/Cargo.toml | 1 - crates/omnigraph-server/src/api.rs | 70 --- crates/omnigraph-server/src/config.rs | 200 +------ crates/omnigraph-server/src/lib.rs | 318 ++--------- crates/omnigraph-server/src/registry.rs | 7 +- crates/omnigraph-server/tests/openapi.rs | 27 +- crates/omnigraph-server/tests/server.rs | 599 +------------------- docs/releases/v0.7.0.md | 47 +- docs/user/cli.md | 19 +- docs/user/policy.md | 17 +- docs/user/server.md | 34 +- openapi.json | 192 ------- 18 files changed, 136 insertions(+), 1727 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b11134d..8147144 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ OmniGraph is a typed property-graph engine built as a coordination layer over ma - **Multi-modal querying**: vector ANN (`nearest`), full-text (`search`/`fuzzy`/`match_text`/`bm25`), Reciprocal Rank Fusion (`rrf`), and graph traversal (`Expand`, anti-join `not { … }`) in one runtime. - **Branches and commits across the whole graph**: Git-style — every successful publish appends to a commit DAG; merges are three-way at the row level. - **Atomic per-query writes**: `mutate_as` and `load` accumulate insert/update batches into an in-memory `MutationStaging.pending` per touched table; one `stage_*` + `commit_staged` per table runs at end-of-query, then `ManifestBatchPublisher::publish` commits the manifest atomically with per-table `expected_table_versions` CAS. A mid-query failure leaves Lance HEAD untouched on staged tables — no drift, no run state machine, no staging branches. Deletes still inline-commit; D₂ at parse time prevents inserts/updates and deletes from coexisting in one query. -- **HTTP server**: Axum + utoipa OpenAPI, bearer auth (SHA-256 hashed, optional AWS Secrets Manager). Cedar policy enforcement is engine-wide — every `_as` writer calls `Omnigraph::enforce(action, scope, actor)`, so HTTP, CLI, and embedded SDK consumers all hit the same gate. **Two modes** (v0.7.0+): single-graph (legacy flat routes) and multi-graph (`/graphs/{graph_id}/...` cluster routes + `POST/GET /graphs` management endpoints with atomic YAML rewrite + drift detection). Per-graph + server-level Cedar policies. +- **HTTP server**: Axum + utoipa OpenAPI, bearer auth (SHA-256 hashed, optional AWS Secrets Manager). Cedar policy enforcement is engine-wide — every `_as` writer calls `Omnigraph::enforce(action, scope, actor)`, so HTTP, CLI, and embedded SDK consumers all hit the same gate. **Two modes** (v0.7.0+): single-graph (legacy flat routes) and multi-graph (`/graphs/{graph_id}/...` cluster routes + read-only `GET /graphs` enumeration). Per-graph + server-level Cedar policies. Runtime add/remove (`POST /graphs`, `DELETE /graphs/{id}`) is not exposed — operators edit `omnigraph.yaml` and restart. - **CLI** driven by a single `omnigraph.yaml`; multi-format output (json/jsonl/csv/kv/table). Throughout the docs, capabilities are split into **L1 — Inherited from Lance** vs **L2 — Added by OmniGraph**. @@ -227,7 +227,7 @@ omnigraph policy explain --actor act-alice --action change --branch main | Three-way row-level merge | — | `OrderedTableCursor` + `StagedTableWriter`, structured `MergeConflictKind` | | Change feeds | — | `diff_between` / `diff_commits` with manifest fast path + ID streaming | | Cedar policy | — | 8 actions, branch / target_branch / protected scopes, validate/test/explain CLI. **Engine-wide enforcement** (MR-722): every `_as` writer (`apply_schema_as`, `mutate_as`, `load_as`, `ingest_as`, `branch_create_as` / `branch_create_from_as`, `branch_delete_as`, `branch_merge_as`) calls `Omnigraph::enforce(action, scope, actor)` — HTTP, CLI, embedded SDK all hit the same gate. | -| HTTP server | — | Axum, OpenAPI via utoipa, bearer auth (SHA-256, AWS Secrets Manager option), `authorize_request` at the HTTP boundary (resolves bearer→actor, applies admission control), NDJSON streaming export, **multi-graph mode (v0.7.0+) with cluster routes + `POST/GET /graphs` management endpoints + atomic YAML rewrite under `fs2::flock` + SHA-256 drift detection** | +| HTTP server | — | Axum, OpenAPI via utoipa, bearer auth (SHA-256, AWS Secrets Manager option), `authorize_request` at the HTTP boundary (resolves bearer→actor, applies admission control), NDJSON streaming export, **multi-graph mode (v0.7.0+) with cluster routes + read-only `GET /graphs` enumeration + per-graph + server-level Cedar policies. Add/remove graphs by editing `omnigraph.yaml` and restarting.** | | CLI with config | — | `omnigraph.yaml`, aliases, multi-format output (json/jsonl/csv/kv/table) | | Audit / actor tracking | — | `_as` write APIs + actor map in commit graph | | Local RustFS bootstrap | — | `scripts/local-rustfs-bootstrap.sh` one-shot S3-backed dev environment | diff --git a/Cargo.lock b/Cargo.lock index b52e218..e95676b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2570,16 +2570,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "fs2" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "fs_extra" version = "1.3.0" @@ -4660,7 +4650,6 @@ dependencies = [ "clap", "color-eyre", "dashmap", - "fs2", "futures", "lance", "lance-index", @@ -7104,22 +7093,6 @@ dependencies = [ "rustls-pki-types", ] -[[package]] -name = "winapi" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" -dependencies = [ - "winapi-i686-pc-windows-gnu", - "winapi-x86_64-pc-windows-gnu", -] - -[[package]] -name = "winapi-i686-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" - [[package]] name = "winapi-util" version = "0.1.11" @@ -7129,12 +7102,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "winapi-x86_64-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" - [[package]] name = "windows-core" version = "0.62.2" diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 4ea6f28..d0324ed 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -18,9 +18,8 @@ use omnigraph_compiler::{ use omnigraph_server::api::{ BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput, BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput, - CommitOutput, ErrorOutput, ExportRequest, GraphCreateRequest, GraphCreateResponse, - GraphListResponse, GraphPolicySpec, GraphSchemaSpec, IngestOutput, IngestRequest, ReadOutput, - ReadRequest, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput, SnapshotOutput, + CommitOutput, ErrorOutput, ExportRequest, GraphListResponse, IngestOutput, IngestRequest, + ReadOutput, ReadRequest, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput, SnapshotOutput, SnapshotTableOutput, commit_output, ingest_output, read_output, schema_apply_output, snapshot_payload, }; @@ -265,13 +264,9 @@ enum Command { /// Operations on the graph registry of a multi-graph server (MR-668). /// /// All operations target a remote multi-graph server URL (http:// or -/// https://). Local-URI invocations return a clear error; for local -/// graphs operators add/remove entries by editing `omnigraph.yaml` -/// directly and restarting. -/// -/// `Delete` is intentionally omitted in v0.7.0 — server-side DELETE -/// was deferred to bound the release scope. Operators remove graphs -/// by stopping the server, editing `omnigraph.yaml`, then restarting. +/// https://). Local-URI invocations return a clear error. To add or +/// remove graphs, operators edit `omnigraph.yaml` directly and restart +/// the server — runtime mutation is not exposed in v0.7.0. #[derive(Debug, Subcommand)] enum GraphsCommand { /// List every graph registered with the multi-graph server. @@ -286,38 +281,6 @@ enum GraphsCommand { #[arg(long)] json: bool, }, - /// Create a new graph at runtime via `POST /graphs`. - /// - /// The schema file is read locally and the bytes are inlined as - /// `schema.source` in the request body. The server runs - /// `Omnigraph::init` at the supplied `uri` and atomically rewrites - /// `omnigraph.yaml` to include the new entry. - Create { - /// Remote server URL (e.g. `https://server.example.com`). - #[arg(long)] - uri: Option, - #[arg(long)] - target: Option, - #[arg(long)] - config: Option, - /// New graph identifier. Must satisfy `^[a-zA-Z0-9-]{1,64}$`. - #[arg(long)] - graph_id: String, - /// Storage URI for the new graph (local path or `s3://...`). - /// Operator-supplied; the server `Omnigraph::init`s here. - #[arg(long = "graph-uri")] - graph_uri: String, - /// Local path to the schema `.pg` file. CLI reads the file - /// and inlines its contents as `schema.source` in the body. - #[arg(long)] - schema: PathBuf, - /// Optional per-graph policy file path. Sent verbatim to the - /// server, where it must be readable at request time. - #[arg(long)] - policy_file: Option, - #[arg(long)] - json: bool, - }, } #[derive(Debug, Subcommand)] @@ -2655,59 +2618,6 @@ async fn main() -> Result<()> { } } } - GraphsCommand::Create { - uri, - target, - config, - graph_id, - graph_uri, - schema, - policy_file, - json, - } => { - let config = load_cli_config(config.as_ref())?; - let bearer_token = - resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?; - let uri = resolve_uri(&config, uri, target.as_deref())?; - if !is_remote_uri(&uri) { - bail!( - "`omnigraph graphs create` requires a remote multi-graph server URL \ - (http:// or https://). To add a graph to a local config, edit \ - `omnigraph.yaml` and restart the server." - ); - } - let schema_source = fs::read_to_string(&schema).map_err(|err| { - color_eyre::eyre::eyre!( - "reading schema file '{}': {err}", - schema.display() - ) - })?; - let request_body = GraphCreateRequest { - graph_id: graph_id.clone(), - uri: graph_uri.clone(), - schema: GraphSchemaSpec { - source: schema_source, - }, - policy: policy_file - .as_ref() - .map(|file| GraphPolicySpec { - file: Some(file.clone()), - }), - }; - let payload = remote_json::( - &http_client, - Method::POST, - remote_url(&uri, "/graphs"), - Some(serde_json::to_value(&request_body)?), - bearer_token.as_deref(), - ) - .await?; - if json { - print_json(&payload)?; - } else { - println!("created graph {} at {}", payload.graph_id, payload.uri); - } - } }, } Ok(()) diff --git a/crates/omnigraph-cli/tests/cli.rs b/crates/omnigraph-cli/tests/cli.rs index 7bb6a16..6502e72 100644 --- a/crates/omnigraph-cli/tests/cli.rs +++ b/crates/omnigraph-cli/tests/cli.rs @@ -2043,25 +2043,26 @@ fn schema_plan_parity_cli_and_sdk() { // ─── MR-668 PR 8 — omnigraph graphs subcommand ───────────────────────────── -/// `omnigraph graphs --help` lists the two subcommands shipped in -/// v0.7.0 (`list`, `create`). `delete` is intentionally NOT in the -/// help — DELETE was deferred to bound the v0.7.0 scope. +/// `omnigraph graphs --help` lists only the read-only `list` +/// subcommand. Runtime add (`create`) and remove (`delete`) are +/// deferred — operators add/remove graphs by editing `omnigraph.yaml` +/// and restarting. This test pins the deferral against accidental +/// re-introduction. #[test] -fn graphs_subcommand_help_lists_list_and_create() { +fn graphs_subcommand_help_lists_list_only() { let output = output_success(cli().arg("graphs").arg("--help")); let stdout = stdout_string(&output); assert!( stdout.contains("list"), "expected `list` subcommand in help output:\n{stdout}" ); + let lowered = stdout.to_lowercase(); assert!( - stdout.contains("create"), - "expected `create` subcommand in help output:\n{stdout}" + !lowered.contains("create a new graph"), + "graph create should not be in v0.7.0 help; got:\n{stdout}" ); - // Pin the deferral: `delete` must not appear yet (catches an - // accidental scope expansion). assert!( - !stdout.to_lowercase().contains("delete a graph"), + !lowered.contains("delete a graph"), "graph delete should not be in v0.7.0 help; got:\n{stdout}" ); } @@ -2078,53 +2079,3 @@ fn graphs_list_against_local_uri_errors_with_remote_only_message() { ); } -/// `omnigraph graphs create` against a local URI errors the same way. -#[test] -fn graphs_create_against_local_uri_errors_with_remote_only_message() { - let temp = tempdir().unwrap(); - let schema = temp.path().join("schema.pg"); - fs::write(&schema, "node Person { name: String @key }\n").unwrap(); - let output = output_failure( - cli() - .arg("graphs") - .arg("create") - .arg("--uri") - .arg("/tmp/local") - .arg("--graph-id") - .arg("alpha") - .arg("--graph-uri") - .arg("/tmp/alpha.omni") - .arg("--schema") - .arg(&schema), - ); - let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); - assert!( - stderr.contains("remote multi-graph server URL"), - "expected 'remote multi-graph server URL' rejection in stderr; got:\n{stderr}" - ); -} - -/// `omnigraph graphs create` with a missing `--schema` file errors -/// with the IO context. Catches a regression where the CLI silently -/// sent an empty body. -#[test] -fn graphs_create_with_missing_schema_file_errors() { - let output = output_failure( - cli() - .arg("graphs") - .arg("create") - .arg("--uri") - .arg("http://127.0.0.1:0") - .arg("--graph-id") - .arg("alpha") - .arg("--graph-uri") - .arg("/tmp/alpha.omni") - .arg("--schema") - .arg("/this/path/does/not/exist.pg"), - ); - let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); - assert!( - stderr.contains("reading schema file"), - "expected 'reading schema file' error in stderr; got:\n{stderr}" - ); -} diff --git a/crates/omnigraph-cli/tests/system_remote.rs b/crates/omnigraph-cli/tests/system_remote.rs index f532564..46f29ec 100644 --- a/crates/omnigraph-cli/tests/system_remote.rs +++ b/crates/omnigraph-cli/tests/system_remote.rs @@ -889,24 +889,21 @@ query insert_person($name: String, $age: I32) { assert_eq!(verify["rows"][0]["p.name"], "PolicyRemote"); } -// ─── MR-668 PR 8 — omnigraph graphs end-to-end ────────────────────────────── +// ─── MR-668 PR 8 — omnigraph graphs list end-to-end ──────────────────────── -/// Multi-graph server + CLI `omnigraph graphs list` / `create` end-to-end. +/// Multi-graph server + CLI `omnigraph graphs list` end-to-end. /// /// Steps: /// 1. Init a graph `alpha` on disk and write an `omnigraph.yaml` /// whose `graphs:` map references it. /// 2. Spawn the server with `--config `. /// 3. `omnigraph graphs list` — expect to see `alpha`. -/// 4. `omnigraph graphs create --graph-id beta --schema ...` — -/// expect 201 and stdout reflecting the new graph. -/// 5. `omnigraph graphs list` again — expect both `alpha` and `beta`. /// /// Ignored by default — spawning servers needs loopback socket /// permissions some sandboxes lack. #[test] #[ignore = "requires loopback socket permissions in sandboxed runners"] -fn graphs_list_and_create_against_multi_graph_server() { +fn graphs_list_against_multi_graph_server() { let cfg_dir = tempfile::tempdir().unwrap(); let schema_path = fixture("test.pg"); @@ -956,7 +953,7 @@ cli: ) .unwrap(); - // 1. `graphs list` lists `alpha`. + // `graphs list` lists `alpha`. let payload = parse_stdout_json(&output_success( cli() .arg("graphs") @@ -965,57 +962,13 @@ cli: .arg(&client_config_path) .arg("--json"), )); - let initial_ids: Vec<&str> = payload["graphs"] + let ids: Vec<&str> = payload["graphs"] .as_array() .unwrap() .iter() .map(|g| g["graph_id"].as_str().unwrap()) .collect(); - assert_eq!(initial_ids, vec!["alpha"]); - - // 2. `graphs create` adds `beta`. - let beta_uri = cfg_dir.path().join("beta.omni"); - let created = parse_stdout_json(&output_success( - cli() - .arg("graphs") - .arg("create") - .arg("--config") - .arg(&client_config_path) - .arg("--graph-id") - .arg("beta") - .arg("--graph-uri") - .arg(beta_uri.to_str().unwrap()) - .arg("--schema") - .arg(&schema_path) - .arg("--json"), - )); - assert_eq!(created["graph_id"], "beta"); - - // 3. `graphs list` now lists both, sorted alphabetically. - let payload = parse_stdout_json(&output_success( - cli() - .arg("graphs") - .arg("list") - .arg("--config") - .arg(&client_config_path) - .arg("--json"), - )); - let final_ids: Vec<&str> = payload["graphs"] - .as_array() - .unwrap() - .iter() - .map(|g| g["graph_id"].as_str().unwrap()) - .collect(); - assert_eq!(final_ids, vec!["alpha", "beta"]); - - // 4. The new graph is reachable via its cluster snapshot route. - let client = Client::new(); - let snap_status = client - .get(format!("{}/graphs/beta/snapshot?branch=main", server.base_url)) - .send() - .unwrap() - .status(); - assert_eq!(snap_status.as_u16(), 200); + assert_eq!(ids, vec!["alpha"]); drop(server); } diff --git a/crates/omnigraph-policy/src/lib.rs b/crates/omnigraph-policy/src/lib.rs index dea80c6..eaec378 100644 --- a/crates/omnigraph-policy/src/lib.rs +++ b/crates/omnigraph-policy/src/lib.rs @@ -39,9 +39,9 @@ pub enum PolicyAction { /// future shape. Avoid writing such rules until the first consumer /// endpoint ships to prevent confusion. Admin, - /// MR-668: management actions that operate on the server's graph - /// registry, not on a single graph's contents. Cedar `appliesTo` - /// declarations bind these to `resource: Server` instead of the + /// MR-668: management action that operates on the server's graph + /// registry, not on a single graph's contents. The Cedar `appliesTo` + /// declaration binds it to `resource: Server` instead of the /// per-graph `resource: Graph`. Operators authorize a group with: /// ```yaml /// rules: @@ -51,12 +51,10 @@ pub enum PolicyAction { /// actions: [graph_list] /// ``` /// `branch_scope` and `target_branch_scope` are NOT supported for - /// these actions — there's no branch context at the server level. - /// `graph_delete` is intentionally omitted from PR 6a; it lands - /// alongside `DELETE /graphs/{id}` in a future release. - GraphCreate, - /// See `GraphCreate`. Currently the only `Server`-scoped action - /// wired into an HTTP endpoint (`GET /graphs`). + /// this action — there's no branch context at the server level. + /// Runtime `graph_create` / `graph_delete` are intentionally omitted + /// from v0.7.0; operators add and remove graphs by editing + /// `omnigraph.yaml` and restarting. GraphList, } @@ -71,7 +69,6 @@ impl PolicyAction { Self::BranchDelete => "branch_delete", Self::BranchMerge => "branch_merge", Self::Admin => "admin", - Self::GraphCreate => "graph_create", Self::GraphList => "graph_list", } } @@ -89,12 +86,12 @@ impl PolicyAction { /// Which Cedar resource entity governs this action. /// Per-graph actions (Read, Change, …) apply to `Omnigraph::Graph::""`. - /// Server-scoped management actions (GraphCreate, GraphList) apply to + /// Server-scoped management actions (GraphList) apply to /// `Omnigraph::Server::"root"`. `Admin` is reserved without a current /// call site; classified as per-graph until MR-724 picks a shape. pub fn resource_kind(self) -> PolicyResourceKind { match self { - Self::GraphCreate | Self::GraphList => PolicyResourceKind::Server, + Self::GraphList => PolicyResourceKind::Server, Self::Read | Self::Export | Self::Change @@ -137,7 +134,6 @@ impl FromStr for PolicyAction { "branch_delete" => Ok(Self::BranchDelete), "branch_merge" => Ok(Self::BranchMerge), "admin" => Ok(Self::Admin), - "graph_create" => Ok(Self::GraphCreate), "graph_list" => Ok(Self::GraphList), other => bail!("unknown policy action '{other}'"), } @@ -329,7 +325,7 @@ impl PolicyConfig { } if server_scoped && graph_scoped { bail!( - "policy rule '{}' mixes server-scoped actions (graph_create, graph_list) \ + "policy rule '{}' mixes the server-scoped action `graph_list` \ with per-graph actions; split into separate rules", rule.id ); @@ -432,9 +428,9 @@ impl PolicyEngine { let principal = entity_uid("Actor", &request.actor_id)?; let action = entity_uid("Action", request.action.as_str())?; // MR-668 PR 6a: pick the resource entity based on the action's - // `resource_kind`. Server-scoped actions (`graph_create`, - // `graph_list`) bind to `Omnigraph::Server::"root"`; per-graph - // actions bind to `Omnigraph::Graph::""`. + // `resource_kind`. Server-scoped actions (`graph_list`) bind to + // `Omnigraph::Server::"root"`; per-graph actions bind to + // `Omnigraph::Graph::""`. let resource = match request.action.resource_kind() { PolicyResourceKind::Server => entity_uid("Server", SERVER_RESOURCE_ID)?, PolicyResourceKind::Graph => entity_uid("Graph", &self.graph_id)?, @@ -701,8 +697,8 @@ fn target_branch_scope_condition(scope: PolicyBranchScope) -> String { } fn policy_schema_source() -> &'static str { - // MR-668 PR 6a: `entity Server;` plus `graph_create`/`graph_list` - // actions that bind to it. Per-graph actions stay bound to `Graph`. + // MR-668 PR 6a: `entity Server;` plus the `graph_list` action that + // binds to it. Per-graph actions stay bound to `Graph`. // The Cedar schema string lives here (not on a fixture file) so any // omnigraph-policy build picks up the new vocabulary in lock-step // with the Rust code. @@ -731,7 +727,6 @@ namespace Omnigraph { action "branch_merge" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; action "admin" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; - action "graph_create" appliesTo { principal: Actor, resource: Server, context: RequestContext }; action "graph_list" appliesTo { principal: Actor, resource: Server, context: RequestContext }; } "# @@ -1132,7 +1127,7 @@ rules: assert!(!deny.allowed); } - // ─── MR-668 PR 6a — server-scoped actions (graph_create, graph_list) ─ + // ─── MR-668 PR 6a — server-scoped action (graph_list) ─ #[test] fn graph_list_action_authorizes_against_server_resource() { @@ -1180,36 +1175,6 @@ rules: assert!(!deny.allowed); } - #[test] - fn graph_create_action_authorizes_against_server_resource() { - let policy: PolicyConfig = serde_yaml::from_str( - r#" -version: 1 -groups: - admins: [act-andrew] -rules: - - id: admins-create-graphs - allow: - actors: { group: admins } - actions: [graph_create, graph_list] -"#, - ) - .unwrap(); - let engine = PolicyCompiler::compile(&policy, "ignored").unwrap(); - - for action in [PolicyAction::GraphCreate, PolicyAction::GraphList] { - let allow = engine - .authorize(&PolicyRequest { - actor_id: "act-andrew".to_string(), - action, - branch: None, - target_branch: None, - }) - .unwrap(); - assert!(allow.allowed, "act-andrew must be allowed {action}"); - } - } - #[test] fn server_scoped_rule_cannot_use_branch_scope() { let policy: PolicyConfig = serde_yaml::from_str( diff --git a/crates/omnigraph-server/Cargo.toml b/crates/omnigraph-server/Cargo.toml index 590095c..7a9d28b 100644 --- a/crates/omnigraph-server/Cargo.toml +++ b/crates/omnigraph-server/Cargo.toml @@ -39,7 +39,6 @@ subtle = { workspace = true } async-trait = { workspace = true } arc-swap = { workspace = true } dashmap = "6" -fs2 = { workspace = true } regex = { workspace = true } thiserror = { workspace = true } aws-config = { version = "1", optional = true, default-features = false, features = ["rustls", "rt-tokio", "credentials-process", "sso"] } diff --git a/crates/omnigraph-server/src/api.rs b/crates/omnigraph-server/src/api.rs index 52c0a33..c59d036 100644 --- a/crates/omnigraph-server/src/api.rs +++ b/crates/omnigraph-server/src/api.rs @@ -488,73 +488,3 @@ pub struct GraphListResponse { pub graphs: Vec, } -// ─── MR-668 PR 7 — POST /graphs request/response ─────────────────────────── - -/// Schema specification for a new graph in `POST /graphs`. Nested -/// per MR-668 decision 7 — leaves room for future fields without -/// breaking the request shape. Mirrors the `policy: { file }` nesting -/// pattern. -/// -/// Today only `source` (inline `.pg` text) is supported. Future fields -/// might include `schema.allow_data_loss`, `schema.version`, etc. -/// -/// **Asymmetric with `SchemaApplyRequest`**: `POST /schema/apply` still -/// uses a flat `schema_source: String` for backwards compatibility. -/// A follow-up release may migrate that too. -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] -pub struct GraphSchemaSpec { - /// Inline `.pg` schema source. - #[schema(example = "node Person {\n name: String @key\n age: I32?\n}")] - pub source: String, -} - -/// Per-graph policy specification in `POST /graphs`. Mirrors the -/// `policy: { file }` shape in `omnigraph.yaml`'s `graphs..policy` -/// section. -#[derive(Debug, Clone, Default, Serialize, Deserialize, ToSchema)] -pub struct GraphPolicySpec { - /// Path to the per-graph Cedar policy file, server-side. - /// Must be readable by the server process at request time. - /// Path is relative to the server's working directory (NOT to the - /// `omnigraph.yaml`'s `base_dir`) — caller-supplied paths are - /// trusted as-is. - pub file: Option, -} - -/// Request body for `POST /graphs` (MR-668 PR 7). -/// -/// Body shape: -/// ```json -/// { -/// "graph_id": "alpha", -/// "uri": "/path/to/alpha.omni", -/// "schema": { "source": "" }, -/// "policy": { "file": "./policies/alpha.yaml" } -/// } -/// ``` -/// -/// 32 MiB body limit (matches `INGEST_REQUEST_BODY_LIMIT_BYTES`). -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] -pub struct GraphCreateRequest { - /// New graph's id. Must satisfy `^[a-zA-Z0-9-]{1,64}$`, not start with - /// `_`, and not be a reserved name. See `GraphId::try_from`. - pub graph_id: String, - /// Storage URI (local path or `s3://...`). Must NOT already be in - /// use by another registered graph. Server `Omnigraph::init`s the - /// graph at this URI. - pub uri: String, - /// Inline schema (`{ source }`). Required. - pub schema: GraphSchemaSpec, - /// Per-graph Cedar policy. Optional — `None` means the graph has - /// no per-graph policy enforcement (HTTP auth still applies if - /// configured). - #[serde(default)] - pub policy: Option, -} - -/// Response from `POST /graphs` on success (201 Created). -#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] -pub struct GraphCreateResponse { - pub graph_id: String, - pub uri: String, -} diff --git a/crates/omnigraph-server/src/config.rs b/crates/omnigraph-server/src/config.rs index 965700b..66ac364 100644 --- a/crates/omnigraph-server/src/config.rs +++ b/crates/omnigraph-server/src/config.rs @@ -5,9 +5,8 @@ use std::path::{Path, PathBuf}; use clap::ValueEnum; use color_eyre::eyre::{Result, bail}; -use fs2::FileExt; use serde::{Deserialize, Serialize}; -use sha2::{Digest, Sha256}; + pub const DEFAULT_CONFIG_FILE: &str = "omnigraph.yaml"; #[derive(Debug, Clone, Default, Serialize, Deserialize)] @@ -68,9 +67,9 @@ pub struct ServerDefaults { pub graph: Option, pub bind: Option, /// Server-level Cedar policy (MR-668). Governs management endpoints - /// (`POST /graphs`, `GET /graphs`, `DELETE /graphs/{id}` once they - /// land). In single-graph mode this is unused — the top-level - /// `policy.file` covers the single graph. + /// — currently `GET /graphs`; future runtime add/remove endpoints + /// will plug in here too. In single-graph mode this is unused — the + /// top-level `policy.file` covers the single graph. #[serde(default)] pub policy: PolicySettings, } @@ -373,197 +372,6 @@ fn absolute_base_dir(cwd: &Path, path: &Path) -> Result { .unwrap_or_else(|| cwd.to_path_buf())) } -/// SHA-256 hash of the file at `path`. Used to baseline `omnigraph.yaml` -/// at server startup; later compared inside `rewrite_atomic` to detect -/// operator hand-edits ("YAML drift") that would otherwise be clobbered -/// silently. Read errors propagate so startup fails loudly if the -/// config file disappears between `load_config` and the hashing. -pub fn hash_config_file(path: &Path) -> std::io::Result<[u8; 32]> { - let bytes = fs::read(path)?; - let digest = Sha256::digest(&bytes); - let mut out = [0u8; 32]; - out.copy_from_slice(&digest); - Ok(out) -} - -/// Why `rewrite_atomic` refused to rewrite. -#[derive(Debug, thiserror::Error)] -pub enum RewriteAtomicError { - /// The on-disk file no longer matches the expected hash — an - /// operator hand-edited `omnigraph.yaml` between server start - /// and now. Rewriting would clobber their changes; instead we - /// refuse loudly. Maps to HTTP 503. - #[error( - "omnigraph.yaml drift detected: on-disk file does not match the server's startup baseline. \ - Stop the server, reconcile the edits, then restart." - )] - Drift, - /// IO failure during the rewrite — couldn't acquire flock, couldn't - /// write the staging file, couldn't rename, etc. The on-disk file - /// is unchanged (rename is atomic on POSIX). Maps to HTTP 500. - #[error("{0}")] - Io(#[from] std::io::Error), - /// Failed to serialize the new `OmnigraphConfig` to YAML. Should - /// not happen in practice — `OmnigraphConfig` has no infallible - /// serde paths in the current types. Maps to HTTP 500. - #[error("serialize config: {0}")] - Serialize(#[from] serde_yaml::Error), -} - -/// Atomically rewrite `omnigraph.yaml` under an exclusive `fcntl::flock` -/// with SHA-256 drift detection (MR-668 PR 7). -/// -/// Returns the new file's hash on success — callers update their -/// in-memory baseline to this value before releasing other request -/// handlers. -/// -/// Sequence (everything inside the flock): -/// 1. Acquire `LOCK_EX` on `path`. -/// 2. Re-read on-disk bytes, hash them. -/// 3. If on-disk hash != `expected_hash` → `RewriteAtomicError::Drift`. -/// 4. Serialize `new_config` to YAML. -/// 5. Write to `path.tmp` and `sync_all` it. -/// 6. `rename(path.tmp, path)` (atomic on POSIX). -/// 7. `sync_all` the parent directory for crash-durability. -/// 8. Release flock (RAII drop on the File). -/// -/// Sync I/O throughout — callers wrap in `tokio::task::spawn_blocking` -/// so the async runtime doesn't stall. -/// -/// **Comments are stripped.** `serde_yaml::to_string` produces canonical -/// YAML without preserving the operator's comments. Decision Q20 in the -/// MR-668 plan accepts this tradeoff for v0.7.0; a future split-file -/// design (`omnigraph.yaml` operator-owned + `omnigraph.runtime.yaml` -/// server-owned) is the escalation path if operators push back. -pub fn rewrite_atomic( - path: &Path, - new_config: &OmnigraphConfig, - expected_hash: &[u8; 32], -) -> std::result::Result<[u8; 32], RewriteAtomicError> { - // 1. flock. Open RW so flock works; we re-read via fs::read below. - let lock_file = fs::OpenOptions::new() - .read(true) - .write(true) - .open(path)?; - lock_file.lock_exclusive()?; - // RAII unlock via `_lock_guard` — the file dropping releases the flock. - let _lock_guard = lock_file; - - // 2. Re-read + hash. - let current_bytes = fs::read(path)?; - let mut current_hash = [0u8; 32]; - current_hash.copy_from_slice(&Sha256::digest(¤t_bytes)); - - // 3. Drift check. - if current_hash != *expected_hash { - return Err(RewriteAtomicError::Drift); - } - - // 4. Serialize new config. - let serialized = serde_yaml::to_string(new_config)?; - - // 5. Write to .tmp + fsync. - let tmp_path = staging_path(path); - fs::write(&tmp_path, &serialized)?; - let tmp_file = fs::File::open(&tmp_path)?; - tmp_file.sync_all()?; - drop(tmp_file); - - // 6. Atomic rename. - fs::rename(&tmp_path, path)?; - - // 7. fsync parent dir for crash-durability (POSIX rename isn't - // durable until the directory entry is synced). - if let Some(parent) = path.parent() { - let dir = fs::File::open(parent)?; - dir.sync_all()?; - } - - // Compute the new file's hash for the caller to update its baseline. - let mut new_hash = [0u8; 32]; - new_hash.copy_from_slice(&Sha256::digest(serialized.as_bytes())); - Ok(new_hash) -} - -/// Staging path used during `rewrite_atomic`: `.tmp` to avoid -/// colliding with any other workflow that might be reading the file. -fn staging_path(path: &Path) -> PathBuf { - let mut s = path.as_os_str().to_owned(); - s.push(".tmp"); - PathBuf::from(s) -} - -/// Atomic read-modify-write of `omnigraph.yaml` (MR-668 PR 7 — race-fix -/// from PR 9). Everything happens **inside** the `fcntl::flock` and the -/// in-memory baseline mutex: -/// 1. Acquire `LOCK_EX`. -/// 2. Lock the in-memory baseline mutex. -/// 3. Read the on-disk file, hash it. -/// 4. Compare to the in-memory baseline; if mismatch → `Drift`. -/// 5. Parse the on-disk YAML, hand the parsed config to `modify`. -/// 6. Serialize the returned config, write `.tmp`, fsync, rename. -/// 7. Update the in-memory baseline to the new file's hash. -/// 8. Release flock + mutex. -/// -/// The earlier `rewrite_atomic` captured the baseline OUTSIDE the -/// flock, which created a race under concurrent writers: a second -/// writer would see a stale baseline + the first writer's new on-disk -/// hash, yielding a spurious `Drift` error. The `_with_modify` shape -/// keeps the entire critical section atomic. -/// -/// `modify` is a `FnOnce` so the caller can read mutable state into it -/// (e.g. a `GraphCreateRequest`) without `Sync` requirements. -pub fn rewrite_atomic_with_modify( - path: &Path, - baseline: &std::sync::Mutex<[u8; 32]>, - modify: F, -) -> std::result::Result<(), RewriteAtomicError> -where - F: FnOnce(OmnigraphConfig) -> std::result::Result, -{ - let lock_file = fs::OpenOptions::new().read(true).write(true).open(path)?; - lock_file.lock_exclusive()?; - let _lock_guard = lock_file; - - // Lock the in-memory baseline INSIDE the flock so concurrent writers - // serialize on both: flock for cross-process safety, mutex for - // in-process baseline updates. The mutex guard outlives the modify - // step so the baseline can't move under our feet. - let mut baseline_guard = baseline - .lock() - .expect("baseline mutex must not be poisoned"); - - let current_bytes = fs::read(path)?; - let mut current_hash = [0u8; 32]; - current_hash.copy_from_slice(&Sha256::digest(¤t_bytes)); - if current_hash != *baseline_guard { - return Err(RewriteAtomicError::Drift); - } - - // Parse the on-disk config (NOT a stale cached version) and hand - // to `modify`. The closure can mutate freely; the result is what - // we serialize and write. - let current_config: OmnigraphConfig = serde_yaml::from_slice(¤t_bytes)?; - let new_config = modify(current_config)?; - let serialized = serde_yaml::to_string(&new_config)?; - - let tmp_path = staging_path(path); - fs::write(&tmp_path, &serialized)?; - let tmp_file = fs::File::open(&tmp_path)?; - tmp_file.sync_all()?; - drop(tmp_file); - fs::rename(&tmp_path, path)?; - if let Some(parent) = path.parent() { - let dir = fs::File::open(parent)?; - dir.sync_all()?; - } - - let mut new_hash = [0u8; 32]; - new_hash.copy_from_slice(&Sha256::digest(serialized.as_bytes())); - *baseline_guard = new_hash; - Ok(()) -} - #[cfg(test)] mod tests { use std::fs; diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index b2e7454..6d0842f 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -82,7 +82,6 @@ fn hash_bearer_token(token: &str) -> BearerTokenHash { paths( server_health, server_graphs_list, - server_graphs_create, server_snapshot, server_read, server_export, @@ -160,13 +159,13 @@ pub enum ServerConfigMode { /// Per-graph startup configs, sorted by graph id (BTreeMap /// iteration order). PR 5's parallel-open loop iterates this. graphs: Vec, - /// Path to the config file the server was started from. PR 7's - /// `POST /graphs` flow will use this for atomic YAML rewrite - /// (DELETE is deferred so the rewrite path is currently unused). + /// Path to the config file the server was started from. Kept on + /// the mode so future runtime mutation (deferred — see release + /// notes) can locate the source of truth without re-parsing CLI + /// args. config_path: PathBuf, /// `server.policy.file` (server-level Cedar policy for the - /// management endpoints). Loaded but currently unused — PR 6b - /// wires it into `GET /graphs`. + /// management endpoints). Wired into `GET /graphs` authorization. server_policy_file: Option, }, } @@ -196,10 +195,9 @@ pub enum ServerMode { /// Backward compatible with v0.6.0 deployments. Single { uri: String }, /// Multi-graph invocation (MR-668). `config_path` is the - /// `omnigraph.yaml` the server reads at startup and (in PR 7, - /// deferred) would rewrite on `POST /graphs`. With DELETE deferred, - /// the current scope does not rewrite the config file — `POST` - /// will (PR 7). + /// `omnigraph.yaml` the server reads at startup. The server treats + /// the file as operator-owned and never writes it; runtime + /// add/remove (deferred) is the only path that would touch it. Multi { config_path: Option }, } @@ -232,16 +230,6 @@ pub struct AppState { /// server policy is configured. Per-graph policies live on each /// `GraphHandle.policy`. server_policy: Option>, - /// PR 7: SHA-256 hash of `omnigraph.yaml` at server startup, used - /// by `POST /graphs` to detect operator hand-edits between server - /// start and the rewrite. Wrapped in `Arc>` so the POST - /// handler can update the baseline after a successful rewrite - /// (later POSTs will compare against the post-rewrite hash, not - /// the original startup hash). - /// - /// `None` in single mode (no config file is rewritten there). Some - /// in multi mode when the server was started with `--config`. - config_hash: Option>>, } struct ExportStreamWriter { @@ -352,7 +340,6 @@ impl AppState { workload: self.workload, bearer_tokens: self.bearer_tokens, server_policy: self.server_policy, - config_hash: self.config_hash, } } @@ -443,7 +430,6 @@ impl AppState { workload, bearer_tokens, server_policy: None, - config_hash: None, } } @@ -453,16 +439,13 @@ impl AppState { /// /// Caller supplies the already-opened `GraphHandle`s and (optionally) /// the path to the source config file. `server_policy` is loaded - /// from `server.policy.file` if configured. `config_hash` is the - /// SHA-256 of `omnigraph.yaml` at startup; `POST /graphs` compares - /// the on-disk file against this baseline before rewriting. + /// from `server.policy.file` if configured. pub fn new_multi( handles: Vec>, bearer_tokens: Vec<(String, String)>, server_policy: Option, workload: workload::WorkloadController, config_path: Option, - config_hash: Option<[u8; 32]>, ) -> std::result::Result { let bearer_tokens = hash_bearer_tokens(bearer_tokens); let registry = Arc::new(GraphRegistry::from_handles(handles)?); @@ -472,7 +455,6 @@ impl AppState { workload: Arc::new(workload), bearer_tokens, server_policy: server_policy.map(Arc::new), - config_hash: config_hash.map(|h| Arc::new(std::sync::Mutex::new(h))), }) } @@ -929,20 +911,18 @@ pub fn build_app(state: AppState) -> Router { require_bearer_auth, )); - // Management endpoints (`GET /graphs`, future `POST /graphs`) - // live alongside the per-graph router. They go through bearer auth - // but NOT through `resolve_graph_handle` — they operate on the - // registry directly. The endpoint is mounted in both modes; in - // single mode the handler returns 405 so clients see "resource - // exists, wrong context" rather than 404 "no such resource." + // Management endpoints (`GET /graphs`) live alongside the per-graph + // router. They go through bearer auth but NOT through + // `resolve_graph_handle` — they operate on the registry directly. + // The endpoint is mounted in both modes; in single mode the handler + // returns 405 so clients see "resource exists, wrong context" + // rather than 404 "no such resource." + // + // Runtime add/remove (`POST /graphs`, `DELETE /graphs/{id}`) is not + // exposed in v0.7.0 — operators add graphs by editing + // `omnigraph.yaml` and restarting. let management = Router::new() - .route( - "/graphs", - get(server_graphs_list).post( - server_graphs_create - .layer(DefaultBodyLimit::max(INGEST_REQUEST_BODY_LIMIT_BYTES)), - ), - ) + .route("/graphs", get(server_graphs_list)) .route_layer(middleware::from_fn_with_state( state.clone(), require_bearer_auth, @@ -1077,11 +1057,6 @@ async fn open_multi_graph_state( .into_iter() .collect::>>()?; - // PR 7: SHA-256 the config file so `POST /graphs` can detect - // operator hand-edits later. - let config_hash = config::hash_config_file(&config_path) - .map_err(|err| color_eyre::eyre::eyre!("hash omnigraph.yaml: {err}"))?; - let workload = workload::WorkloadController::from_env(); let state = AppState::new_multi( handles, @@ -1089,15 +1064,13 @@ async fn open_multi_graph_state( server_policy, workload, Some(config_path), - Some(config_hash), ) .map_err(|err| color_eyre::eyre::eyre!("multi-graph registry: {err}"))?; Ok(state) } -/// Open one graph and wrap it in a `GraphHandle`. Used both at startup -/// (`open_multi_graph_state`) and — once `POST /graphs` lands in PR 7 -/// — for runtime additions. +/// Open one graph and wrap it in a `GraphHandle`. Used at startup by +/// `open_multi_graph_state`. async fn open_single_graph(cfg: GraphStartupConfig) -> Result> { let graph_id = GraphId::try_from(cfg.graph_id.clone()) .map_err(|err| color_eyre::eyre::eyre!("graph id '{}': {err}", cfg.graph_id))?; @@ -1226,237 +1199,6 @@ async fn server_graphs_list( Ok(Json(GraphListResponse { graphs })) } -#[utoipa::path( - post, - path = "/graphs", - tag = "management", - operation_id = "createGraph", - request_body = api::GraphCreateRequest, - responses( - (status = 201, description = "Graph created", body = api::GraphCreateResponse), - (status = 400, description = "Invalid request body (graph_id, schema, policy file)", body = ErrorOutput), - (status = 401, description = "Unauthorized", body = ErrorOutput), - (status = 403, description = "Forbidden", body = ErrorOutput), - (status = 405, description = "Method not allowed (single-graph mode)", body = ErrorOutput), - (status = 409, description = "graph_id or uri already registered", body = ErrorOutput), - (status = 413, description = "Request body too large (>32 MiB)", body = ErrorOutput), - (status = 500, description = "Init failure or YAML rewrite failure", body = ErrorOutput), - (status = 503, description = "omnigraph.yaml drift detected (operator edited the file)", body = ErrorOutput), - ), - security(("bearer_token" = [])), -)] -/// Create a new graph at runtime (MR-668 PR 7). -/// -/// Multi-graph mode only. Operators add a graph to the registry -/// without restarting the server. The server `Omnigraph::init`s the -/// new graph at `req.uri`, atomically rewrites `omnigraph.yaml` to -/// include the new entry, then publishes the handle in the registry. -/// -/// Cedar-gated by `PolicyAction::GraphCreate` against -/// `Omnigraph::Server::"root"` (the same server-level policy as -/// `GET /graphs`). -/// -/// Failure modes: -/// * Init fails → orphan storage files at `req.uri` (PR 2a cleans up -/// schema files but not Lance datasets; operator removes manually). -/// * Rewrite fails (`fs2::flock` IO error) → orphan storage; YAML -/// unchanged. -/// * YAML drift (operator edited the file) → 503; YAML and storage -/// both unchanged. -/// * Duplicate `graph_id` or `uri` → 409; storage already in use. -async fn server_graphs_create( - State(state): State, - actor: Option>, - Json(request): Json, -) -> std::result::Result<(StatusCode, Json), ApiError> { - // ─── 1. Mode check: management endpoints don't apply in single mode. - let (config_path, config_hash) = match state.mode() { - ServerMode::Single { .. } => { - return Err(ApiError { - status: StatusCode::METHOD_NOT_ALLOWED, - code: ErrorCode::BadRequest, - message: "POST /graphs is only available in multi-graph mode".to_string(), - merge_conflicts: Vec::new(), - manifest_conflict: None, - }); - } - ServerMode::Multi { config_path } => match (config_path.clone(), state.config_hash.clone()) { - (Some(path), Some(hash)) => (path, hash), - _ => { - return Err(ApiError::internal( - "multi-mode AppState missing config_path or config_hash".to_string(), - )); - } - }, - }; - - // ─── 2. Cedar authorize. Server-level policy gates this. - authorize_request( - actor.as_ref().map(|Extension(actor)| actor), - state.server_policy.as_deref(), - PolicyRequest { - actor_id: actor - .as_ref() - .map(|Extension(actor)| actor.actor_id.as_ref().to_string()) - .unwrap_or_default(), - action: PolicyAction::GraphCreate, - branch: None, - target_branch: None, - }, - )?; - - // ─── 3. Validate request body. - let graph_id = GraphId::try_from(request.graph_id.clone()) - .map_err(|err| ApiError::bad_request(err.to_string()))?; - if request.schema.source.trim().is_empty() { - return Err(ApiError::bad_request( - "schema.source must not be empty".to_string(), - )); - } - if request.uri.trim().is_empty() { - return Err(ApiError::bad_request("uri must not be empty".to_string())); - } - - // Per-graph policy file (optional). Resolved as caller-supplied path. - // Validation: must exist and parse against the Cedar schema. Loading - // here surfaces config errors before we init the graph. - let policy_file_str = request - .policy - .as_ref() - .and_then(|p| p.file.clone()) - .filter(|s| !s.trim().is_empty()); - let policy_engine = if let Some(path) = policy_file_str.as_deref() { - Some( - PolicyEngine::load(std::path::Path::new(path), graph_id.as_str()) - .map_err(|err| ApiError::bad_request(format!("policy.file: {err}")))?, - ) - } else { - None - }; - - // ─── 4. Pre-check duplicates (best-effort — registry.insert is the - // authoritative atomic check, but this returns a clearer error - // when the duplicate is obvious). - let key = GraphKey::cluster(graph_id.clone()); - if matches!(state.registry().get(&key), RegistryLookup::Ready(_)) { - return Err(ApiError::conflict(format!( - "graph '{graph_id}' is already registered" - ))); - } - if state - .registry() - .list() - .iter() - .any(|h| h.uri == request.uri) - { - return Err(ApiError::conflict(format!( - "uri '{}' is already in use by another graph", - request.uri - ))); - } - - // ─── 5. Init the new engine at the requested URI. PR 2a's cleanup - // removes schema files on init failure; Lance directories - // become orphans if `GraphCoordinator::init` partially - // succeeded (documented limitation pending delete_prefix). - let engine = Omnigraph::init(&request.uri, &request.schema.source) - .await - .map_err(|err| ApiError::internal(format!("init: {err}")))?; - - // Apply engine-layer policy enforcement (MR-722). HTTP-layer is the - // first gate; this is the redundant-but-correct backstop. - let (engine, policy_arc): (Omnigraph, Option>) = if let Some(p) = policy_engine - { - let policy_arc: Arc = Arc::new(p); - let checker = Arc::clone(&policy_arc) as Arc; - (engine.with_policy(checker), Some(policy_arc)) - } else { - (engine, None) - }; - - let handle = Arc::new(GraphHandle { - key: key.clone(), - uri: request.uri.clone(), - engine: Arc::new(engine), - policy: policy_arc, - }); - - // ─── 6. Rewrite omnigraph.yaml atomically (drift detection inside). - // Done in a blocking task because `fs2::flock` is sync. - let new_target = config::TargetConfig { - uri: request.uri.clone(), - bearer_token_env: None, - policy: config::PolicySettings { - file: policy_file_str.clone(), - }, - }; - let graph_id_for_yaml = graph_id.as_str().to_string(); - let config_path_for_blocking = config_path.clone(); - let config_hash_for_blocking = Arc::clone(&config_hash); - let rewrite_result = tokio::task::spawn_blocking(move || { - rewrite_yaml_with_new_graph( - &config_path_for_blocking, - &config_hash_for_blocking, - &graph_id_for_yaml, - new_target, - ) - }) - .await - .map_err(|err| ApiError::internal(format!("rewrite join: {err}")))?; - rewrite_result?; - - // ─── 7. Publish in the registry. If this fails (race), the YAML - // already has the entry — on restart it gets opened and - // added cleanly. Operator-visible inconsistency is brief - // (just until next restart). - state - .registry() - .insert(Arc::clone(&handle)) - .await - .map_err(|err| match err { - registry::InsertError::DuplicateKey(_) | registry::InsertError::DuplicateUri(_) => { - ApiError::conflict(err.to_string()) - } - })?; - - Ok(( - StatusCode::CREATED, - Json(api::GraphCreateResponse { - graph_id: graph_id.as_str().to_string(), - uri: request.uri, - }), - )) -} - -/// Atomically rewrite `omnigraph.yaml` to add a new graph entry. -/// Runs inside `tokio::task::spawn_blocking` (the flock is sync). -/// -/// Read-modify-write happens entirely under the flock + baseline -/// mutex via `config::rewrite_atomic_with_modify` — concurrent -/// writers serialize without spurious drift errors. -fn rewrite_yaml_with_new_graph( - config_path: &std::path::Path, - config_hash: &Arc>, - graph_id: &str, - new_target: config::TargetConfig, -) -> std::result::Result<(), ApiError> { - let graph_id = graph_id.to_string(); - config::rewrite_atomic_with_modify(config_path, config_hash, move |mut config| { - config.graphs.insert(graph_id, new_target); - Ok(config) - }) - .map_err(|err| match err { - config::RewriteAtomicError::Drift => ApiError { - status: StatusCode::SERVICE_UNAVAILABLE, - code: ErrorCode::Conflict, - message: err.to_string(), - merge_conflicts: Vec::new(), - manifest_conflict: None, - }, - other => ApiError::internal(other.to_string()), - }) -} - async fn server_openapi(State(state): State) -> Json { let mut doc = ApiDoc::openapi(); if !state.requires_bearer_auth() { @@ -1482,9 +1224,12 @@ const CLUSTER_PATH_PREFIX: &str = "/graphs/{graph_id}"; /// the same generation pass. const CLUSTER_OPERATION_ID_PREFIX: &str = "cluster_"; -/// Paths that stay flat in every server mode (public, no per-graph -/// dependency). Update this list when adding new always-public endpoints. -const ALWAYS_FLAT_PATHS: &[&str] = &["/healthz"]; +/// Paths that stay flat in every server mode (public or server-level, +/// no per-graph dependency). Update this list when adding new +/// always-flat endpoints. `/graphs` is the management enumeration — +/// it lives at the root in both single mode (405) and multi mode, and +/// must never be rewritten to `/graphs/{graph_id}/graphs`. +const ALWAYS_FLAT_PATHS: &[&str] = &["/healthz", "/graphs"]; /// In multi-mode `server_openapi`, every protected path-item is /// reattached under the cluster prefix. Operation IDs gain the @@ -1671,9 +1416,10 @@ fn log_policy_decision(actor_id: &str, request: &PolicyRequest, decision: &Polic /// HTTP-layer Cedar policy gate. Two sources of the policy engine: /// * Per-graph handler — passes `handle.policy.as_deref()` so the /// graph's Cedar rules govern read/change/branch_*/schema_apply. -/// * Management handler (PR 6b, deferred for now) — passes -/// `state.server_policy.as_deref()` so server-level Cedar rules -/// govern `graph_create`/`graph_list`/`graph_delete`. +/// * Management handler — passes `state.server_policy.as_deref()` so +/// server-level Cedar rules govern `graph_list` (the only shipped +/// server-scoped action; runtime `graph_create` / `graph_delete` +/// are deferred until a managed cluster catalog lands). /// /// The MR-731 invariant lives inside this function: actor identity is /// overwritten from the resolved bearer match, never trusted from the diff --git a/crates/omnigraph-server/src/registry.rs b/crates/omnigraph-server/src/registry.rs index 14e48e7..2140a93 100644 --- a/crates/omnigraph-server/src/registry.rs +++ b/crates/omnigraph-server/src/registry.rs @@ -141,8 +141,11 @@ impl GraphRegistry { } /// Add a new handle. Async because the mutex is `tokio::sync::Mutex` - /// (held across `.await` points in PR 7's atomic-YAML-rewrite flow). - /// Rejects duplicate `GraphKey` and duplicate `uri`. + /// (a future managed-catalog flow may hold it across `.await` points + /// during atomic registry mutations). Rejects duplicate `GraphKey` + /// and duplicate `uri`. Currently unused at runtime — only construction + /// via `from_handles` runs at startup — but kept for the tests that + /// pin its concurrency contract. /// /// Race semantics (pinned by `concurrent_insert_same_key_one_succeeds_one_errors`): /// under two concurrent calls with the same key, exactly one returns diff --git a/crates/omnigraph-server/tests/openapi.rs b/crates/omnigraph-server/tests/openapi.rs index 4451bbc..e93e11b 100644 --- a/crates/omnigraph-server/tests/openapi.rs +++ b/crates/omnigraph-server/tests/openapi.rs @@ -1008,7 +1008,7 @@ async fn app_for_multi_mode(graph_ids: &[&str]) -> (Vec, Rout dirs.push(dir); } let workload = omnigraph_server::workload::WorkloadController::from_env(); - let state = AppState::new_multi(handles, Vec::new(), None, workload, None, None).unwrap(); + let state = AppState::new_multi(handles, Vec::new(), None, workload, None).unwrap(); let app = build_app(state); (dirs, app) } @@ -1069,7 +1069,7 @@ async fn multi_mode_openapi_drops_flat_protected_paths() { } #[tokio::test] -async fn multi_mode_openapi_keeps_healthz_flat() { +async fn multi_mode_openapi_keeps_management_paths_flat() { let (_dirs, app) = app_for_multi_mode(&["alpha"]).await; let request = Request::builder() .method(Method::GET) @@ -1078,14 +1078,17 @@ async fn multi_mode_openapi_keeps_healthz_flat() { .unwrap(); let (_, json) = json_response(&app, request).await; let paths = json["paths"].as_object().unwrap(); - assert!( - paths.contains_key("/healthz"), - "/healthz must remain flat in multi mode" - ); - assert!( - !paths.contains_key("/graphs/{graph_id}/healthz"), - "/healthz must NOT be cluster-prefixed" - ); + for flat in ["/healthz", "/graphs"] { + assert!( + paths.contains_key(flat), + "{flat} must remain flat in multi mode" + ); + let nested = format!("/graphs/{{graph_id}}{flat}"); + assert!( + !paths.contains_key(&nested), + "{flat} must NOT be cluster-prefixed to {nested}" + ); + } } #[tokio::test] @@ -1098,10 +1101,12 @@ async fn multi_mode_openapi_prefixes_operation_ids_with_cluster() { .unwrap(); let (_, json) = json_response(&app, request).await; // Every cluster path operation must have a `cluster_` operation_id. + // Flat-mounted paths (healthz, management /graphs) keep their + // original operation_ids — they're not per-graph. let paths = json["paths"].as_object().unwrap(); let mut checked = 0; for (path, item) in paths { - if path == "/healthz" { + if path == "/healthz" || path == "/graphs" { continue; } for method in ["get", "post", "put", "delete", "patch"] { diff --git a/crates/omnigraph-server/tests/server.rs b/crates/omnigraph-server/tests/server.rs index 6e8f0f2..ed5088d 100644 --- a/crates/omnigraph-server/tests/server.rs +++ b/crates/omnigraph-server/tests/server.rs @@ -4360,7 +4360,7 @@ mod multi_graph_startup { dirs.push(dir); } let workload = omnigraph_server::workload::WorkloadController::from_env(); - let state = AppState::new_multi(handles, Vec::new(), None, workload, None, None).unwrap(); + let state = AppState::new_multi(handles, Vec::new(), None, workload, None).unwrap(); let app = build_app(state); (dirs, app) } @@ -4741,7 +4741,7 @@ graphs: let tokens = vec![("act-andrew".to_string(), "secret-token".to_string())]; let workload = omnigraph_server::workload::WorkloadController::from_env(); let state = - AppState::new_multi(vec![handle], tokens, None, workload, None, None).unwrap(); + AppState::new_multi(vec![handle], tokens, None, workload, None).unwrap(); let app = build_app(state); // No Authorization header → 401. @@ -4822,7 +4822,6 @@ rules: Some(server_policy), workload, None, - None, ) .unwrap(); let app = build_app(state); @@ -4865,333 +4864,6 @@ rules: ); } - // ─── PR 7 — POST /graphs ────────────────────────────────────────── - - use omnigraph_server::api::{GraphCreateRequest, GraphCreateResponse, GraphPolicySpec, GraphSchemaSpec}; - use omnigraph_server::config::{OmnigraphConfig, hash_config_file}; - - /// Spin up a multi-mode server whose `omnigraph.yaml` we control, - /// so PR 7's `POST /graphs` can rewrite it. Returns the config - /// directory (to live across the test) and a built `Router`. - async fn multi_mode_app_with_real_config( - initial_graphs: &[&str], - ) -> (tempfile::TempDir, Router) { - let cfg_dir = tempfile::tempdir().unwrap(); - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - - // Init each starting graph at a real URI inside the config dir. - let mut yaml_graphs = String::new(); - let mut handles = Vec::new(); - for id in initial_graphs { - let graph_uri = cfg_dir.path().join(format!("{id}.omni")); - Omnigraph::init(graph_uri.to_str().unwrap(), &schema) - .await - .unwrap(); - yaml_graphs.push_str(&format!( - " {id}:\n uri: {}\n", - graph_uri.display() - )); - // Open in-memory engine for the handle. - let engine = Omnigraph::open(graph_uri.to_str().unwrap()) - .await - .unwrap(); - handles.push(Arc::new( - omnigraph_server::GraphHandle { - key: omnigraph_server::GraphKey::cluster( - omnigraph_server::GraphId::try_from(*id).unwrap(), - ), - uri: graph_uri.to_string_lossy().to_string(), - engine: Arc::new(engine), - policy: None, - }, - )); - } - let config_path = cfg_dir.path().join("omnigraph.yaml"); - fs::write(&config_path, format!("graphs:\n{yaml_graphs}")).unwrap(); - let config_hash = hash_config_file(&config_path).unwrap(); - - let workload = omnigraph_server::workload::WorkloadController::from_env(); - let state = AppState::new_multi( - handles, - Vec::new(), - None, - workload, - Some(config_path.clone()), - Some(config_hash), - ) - .unwrap(); - let app = build_app(state); - (cfg_dir, app) - } - - async fn post_graph( - app: &Router, - body: &GraphCreateRequest, - auth: Option<&str>, - ) -> (StatusCode, Value) { - let json_body = serde_json::to_vec(body).unwrap(); - let mut request = Request::builder() - .method(Method::POST) - .uri("/graphs") - .header("content-type", "application/json"); - if let Some(token) = auth { - request = request.header("authorization", format!("Bearer {token}")); - } - let req = request.body(Body::from(json_body)).unwrap(); - let response = app.clone().oneshot(req).await.unwrap(); - let status = response.status(); - let body_bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap(); - let body_json: Value = if body_bytes.is_empty() { - Value::Null - } else { - serde_json::from_slice(&body_bytes).unwrap_or(Value::Null) - }; - (status, body_json) - } - - /// Happy path: POST creates a new graph, returns 201, the graph is - /// queryable via cluster routes, and omnigraph.yaml now includes it. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_creates_a_new_graph_end_to_end() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - let new_uri = cfg_dir.path().join("beta.omni"); - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: new_uri.to_string_lossy().to_string(), - schema: GraphSchemaSpec { source: schema }, - policy: None, - }; - let (status, body) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::CREATED, "got body: {body}"); - let resp: GraphCreateResponse = serde_json::from_value(body).unwrap(); - assert_eq!(resp.graph_id, "beta"); - - // The new graph is reachable via its cluster route. - let snap = app - .clone() - .oneshot( - Request::builder() - .method(Method::GET) - .uri("/graphs/beta/snapshot?branch=main") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(snap.status(), StatusCode::OK); - - // The YAML on disk now references the new graph. - let yaml = fs::read_to_string(cfg_dir.path().join("omnigraph.yaml")).unwrap(); - assert!( - yaml.contains("beta:"), - "rewritten YAML must include 'beta:'; got:\n{yaml}" - ); - assert!( - yaml.contains(new_uri.to_str().unwrap()), - "rewritten YAML must include the new URI; got:\n{yaml}" - ); - } - - /// Two POSTs in sequence both succeed: the second one's drift - /// check passes because the first POST updates the in-memory - /// baseline hash to the post-rewrite hash. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_baseline_hash_updates_between_rewrites() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - for name in ["beta", "gamma"] { - let new_uri = cfg_dir.path().join(format!("{name}.omni")); - let req = GraphCreateRequest { - graph_id: name.to_string(), - uri: new_uri.to_string_lossy().to_string(), - schema: GraphSchemaSpec { - source: schema.clone(), - }, - policy: None, - }; - let (status, body) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::CREATED, "create {name}: {body}"); - } - let yaml = fs::read_to_string(cfg_dir.path().join("omnigraph.yaml")).unwrap(); - assert!(yaml.contains("beta:")); - assert!(yaml.contains("gamma:")); - } - - /// Duplicate `graph_id` returns 409. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_duplicate_graph_id_returns_409() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - let req = GraphCreateRequest { - graph_id: "alpha".to_string(), // already registered - uri: cfg_dir - .path() - .join("alpha-duplicate.omni") - .to_string_lossy() - .to_string(), - schema: GraphSchemaSpec { source: schema }, - policy: None, - }; - let (status, body) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::CONFLICT, "got body: {body}"); - } - - /// Duplicate `uri` returns 409. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_duplicate_uri_returns_409() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - let alpha_uri = cfg_dir.path().join("alpha.omni"); - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: alpha_uri.to_string_lossy().to_string(), // already in use - schema: GraphSchemaSpec { source: schema }, - policy: None, - }; - let (status, _) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::CONFLICT); - } - - /// Invalid `graph_id` (reserved name) returns 400. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_invalid_graph_id_returns_400() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - let req = GraphCreateRequest { - graph_id: "policies".to_string(), // reserved - uri: cfg_dir - .path() - .join("policies.omni") - .to_string_lossy() - .to_string(), - schema: GraphSchemaSpec { source: schema }, - policy: None, - }; - let (status, _) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::BAD_REQUEST); - } - - /// Empty schema source returns 400 with a clear message. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_empty_schema_source_returns_400() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: cfg_dir - .path() - .join("beta.omni") - .to_string_lossy() - .to_string(), - schema: GraphSchemaSpec { - source: " \n ".to_string(), - }, - policy: None, - }; - let (status, body) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::BAD_REQUEST); - assert!( - body.to_string().contains("schema.source"), - "expected schema.source rejection in body: {body}" - ); - } - - /// Single mode rejects `POST /graphs` with 405. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_returns_405_in_single_mode() { - let temp = init_loaded_graph().await; - let graph = graph_path(temp.path()); - let state = AppState::open(graph.to_string_lossy().to_string()) - .await - .unwrap(); - let app = build_app(state); - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: "/tmp/beta.omni".to_string(), - schema: GraphSchemaSpec { - source: "node Person { name: String @key }\n".to_string(), - }, - policy: None, - }; - let (status, _) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::METHOD_NOT_ALLOWED); - } - - /// YAML drift detection: operator hand-edits the config file - /// between server start and the POST → 503 Service Unavailable. - #[tokio::test(flavor = "multi_thread")] - async fn post_graphs_yaml_drift_detection_returns_503() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - // Simulate an operator editing the file out from under the - // running server. This changes the on-disk hash; the server's - // in-memory baseline (computed at startup) no longer matches. - let config_path = cfg_dir.path().join("omnigraph.yaml"); - let mut yaml = fs::read_to_string(&config_path).unwrap(); - yaml.push_str("\n# operator added a comment after server start\n"); - fs::write(&config_path, yaml).unwrap(); - - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: cfg_dir - .path() - .join("beta.omni") - .to_string_lossy() - .to_string(), - schema: GraphSchemaSpec { source: schema }, - policy: None, - }; - let (status, body) = post_graph(&app, &req, None).await; - assert_eq!( - status, - StatusCode::SERVICE_UNAVAILABLE, - "expected drift detection, got: {body}" - ); - assert!( - body.to_string().contains("drift"), - "expected drift message, got: {body}" - ); - } - - /// hash_config_file is deterministic and detects byte-level changes. - #[test] - fn hash_config_file_is_deterministic_and_detects_changes() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("cfg.yaml"); - fs::write(&path, "graphs:\n alpha:\n uri: /tmp/a.omni\n").unwrap(); - let h1 = hash_config_file(&path).unwrap(); - let h2 = hash_config_file(&path).unwrap(); - assert_eq!(h1, h2, "hash must be deterministic"); - fs::write(&path, "graphs:\n alpha:\n uri: /tmp/b.omni\n").unwrap(); - let h3 = hash_config_file(&path).unwrap(); - assert_ne!(h1, h3, "hash must change when content changes"); - } - - /// rewrite_atomic refuses to rewrite when the baseline doesn't match. - #[test] - fn rewrite_atomic_refuses_when_hash_drifts() { - use omnigraph_server::config::{RewriteAtomicError, rewrite_atomic}; - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("cfg.yaml"); - fs::write(&path, "graphs:\n alpha:\n uri: /tmp/a.omni\n").unwrap(); - // Pass an obviously-wrong baseline hash. - let wrong_hash = [0u8; 32]; - let mut new_config = OmnigraphConfig::default(); - new_config.graphs.insert( - "beta".to_string(), - omnigraph_server::config::TargetConfig { - uri: "/tmp/b.omni".to_string(), - bearer_token_env: None, - policy: Default::default(), - }, - ); - let err = rewrite_atomic(&path, &new_config, &wrong_hash).unwrap_err(); - assert!( - matches!(err, RewriteAtomicError::Drift), - "expected Drift, got: {err}" - ); - } - /// End-to-end: load an `omnigraph.yaml` with two graphs and serve /// them. Both graphs must be queryable via cluster routes. /// @@ -5244,271 +4916,4 @@ graphs: _ => unreachable!(), } } - - // ─── PR 9: composite lifecycle tests ───────────────────────────────── - // - // These tests exercise PRs 1–8 in combination. Each test composes - // multiple primitives (POST a graph, query it, restart, enforce - // per-graph policy) into a single scenario. They're the closure - // tests for the gaps I flagged in PR 7's coverage assessment — - // not redundant with the per-PR tests because they catch - // integration regressions that individual unit tests miss. - - /// Post a graph, query it via cluster route, then re-load the - /// config from disk and confirm `load_server_settings` sees the - /// rewritten YAML (i.e. the server's `POST /graphs` actually - /// persists). Validates that on restart, the new graph would be - /// opened automatically by `serve()`'s multi-mode startup. - #[tokio::test(flavor = "multi_thread")] - async fn multi_graph_lifecycle_post_query_restart_persistence() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - - // 1. POST a new graph `beta`. - let beta_uri = cfg_dir.path().join("beta.omni"); - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: beta_uri.to_string_lossy().to_string(), - schema: GraphSchemaSpec { - source: schema.clone(), - }, - policy: None, - }; - let (status, _) = post_graph(&app, &req, None).await; - assert_eq!(status, StatusCode::CREATED); - - // 2. Query the new graph via its cluster route. - let snap = app - .clone() - .oneshot( - Request::builder() - .method(Method::GET) - .uri("/graphs/beta/snapshot?branch=main") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(snap.status(), StatusCode::OK); - - // 3. "Restart": reload the config and confirm the rewritten - // YAML carries the new graph through `load_server_settings`. - // A real restart calls `open_multi_graph_state` next; we - // stop short of opening Lance again (the per-PR tests - // already cover that path) but assert the inferred - // `ServerConfigMode::Multi` lists both graphs. - let config_path = cfg_dir.path().join("omnigraph.yaml"); - let settings: ServerConfig = - load_server_settings(Some(&config_path), None, None, None, true).unwrap(); - match settings.mode { - ServerConfigMode::Multi { graphs, .. } => { - let ids: Vec<&str> = graphs.iter().map(|g| g.graph_id.as_str()).collect(); - assert_eq!( - ids, - vec!["alpha", "beta"], - "rewritten YAML must include both graphs in BTreeMap order" - ); - } - _ => panic!("expected Multi mode after restart"), - } - } - - /// Per-graph Cedar policy is enforced for a graph created via POST. - /// Closes the gap from PR 7's test coverage — the policy was loaded - /// but never exercised end-to-end. This test sends an authenticated - /// `change` request against a POST-created graph whose per-graph - /// policy denies `change` for that actor. - #[tokio::test(flavor = "multi_thread")] - async fn per_graph_policy_enforced_on_post_created_graph() { - let (cfg_dir, _initial_app) = multi_mode_app_with_real_config(&[]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - let config_path = cfg_dir.path().join("omnigraph.yaml"); - let config_hash = omnigraph_server::config::hash_config_file(&config_path).unwrap(); - // Server-level policy: act-andrew can create graphs. Required - // because requires_bearer_auth fires under MR-723 default-deny - // once we configure tokens, and `GraphCreate != Read` would - // otherwise 403 without a server policy. - let server_policy_path = cfg_dir.path().join("server-policy.yaml"); - fs::write( - &server_policy_path, - r#" -version: 1 -groups: - admins: [act-andrew] -rules: - - id: admins-create - allow: - actors: { group: admins } - actions: [graph_create, graph_list] -"#, - ) - .unwrap(); - let server_policy = omnigraph_policy::PolicyEngine::load(&server_policy_path, "server") - .unwrap(); - let workload = omnigraph_server::workload::WorkloadController::from_env(); - let state = AppState::new_multi( - vec![], - vec![ - ("act-andrew".to_string(), "andrew-token".to_string()), - ("act-bruno".to_string(), "bruno-token".to_string()), - ], - Some(server_policy), - workload, - Some(config_path.clone()), - Some(config_hash), - ) - .expect("empty multi-mode registry must be constructible"); - let app = build_app(state); - - // Per-graph policy file: only `act-andrew` may `change`. - let beta_policy_path = cfg_dir.path().join("beta-policy.yaml"); - fs::write( - &beta_policy_path, - r#" -version: 1 -groups: - writers: [act-andrew] - readers: [act-bruno] -protected_branches: [] -rules: - - id: writers-change - allow: - actors: { group: writers } - actions: [read, change] - branch_scope: any - - id: readers-read - allow: - actors: { group: readers } - actions: [read] - branch_scope: any -"#, - ) - .unwrap(); - - // POST `beta` with the per-graph policy attached. - let beta_uri = cfg_dir.path().join("beta.omni"); - let req = GraphCreateRequest { - graph_id: "beta".to_string(), - uri: beta_uri.to_string_lossy().to_string(), - schema: GraphSchemaSpec { source: schema }, - policy: Some(omnigraph_server::api::GraphPolicySpec { - file: Some(beta_policy_path.to_string_lossy().to_string()), - }), - }; - let (status, body) = post_graph(&app, &req, Some("andrew-token")).await; - assert_eq!( - status, - StatusCode::CREATED, - "POST /graphs failed: {body}" - ); - - // Authenticated `read` from a reader: 200. - let read_resp = app - .clone() - .oneshot( - Request::builder() - .method(Method::GET) - .uri("/graphs/beta/snapshot?branch=main") - .header("authorization", "Bearer bruno-token") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!( - read_resp.status(), - StatusCode::OK, - "act-bruno must be allowed read on beta" - ); - - // Authenticated `change` from the reader (act-bruno) must 403: - // beta-policy allows readers only `read`, not `change`. - let change_body = serde_json::json!({ - "query_source": "query foo() { insert Person { name: \"X\" } }", - "query_name": "foo", - "branch": "main" - }); - let change_resp = app - .oneshot( - Request::builder() - .method(Method::POST) - .uri("/graphs/beta/change") - .header("authorization", "Bearer bruno-token") - .header("content-type", "application/json") - .body(Body::from(serde_json::to_vec(&change_body).unwrap())) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!( - change_resp.status(), - StatusCode::FORBIDDEN, - "per-graph Cedar policy must deny `change` for act-bruno on beta" - ); - } - - /// Concurrent POST /graphs for DISTINCT graph_ids all succeed. - /// The flock + drift detection serializes the YAML rewrite, but - /// all writes are valid and the final YAML lists every graph. - /// (Same-graph_id concurrency is already covered by the - /// `concurrent_insert_same_key_exactly_one_succeeds` registry - /// test plus the YAML drift-detection behavior.) - #[tokio::test(flavor = "multi_thread")] - async fn concurrent_post_graphs_distinct_ids_all_succeed() { - let (cfg_dir, app) = multi_mode_app_with_real_config(&["alpha"]).await; - let schema = fs::read_to_string(fixture("test.pg")).unwrap(); - const N: usize = 4; - - let app = Arc::new(app); - let barrier = Arc::new(tokio::sync::Barrier::new(N)); - let mut tasks = Vec::with_capacity(N); - for i in 0..N { - let app = Arc::clone(&app); - let barrier = Arc::clone(&barrier); - let dir = cfg_dir.path().to_path_buf(); - let schema = schema.clone(); - tasks.push(tokio::spawn(async move { - barrier.wait().await; - let id = format!("graph-{i}"); - let uri = dir.join(format!("{id}.omni")); - let req = GraphCreateRequest { - graph_id: id.clone(), - uri: uri.to_string_lossy().to_string(), - schema: GraphSchemaSpec { source: schema }, - policy: None, - }; - let (status, _) = post_graph(&app, &req, None).await; - (id, status) - })); - } - - let mut succeeded = Vec::new(); - for t in tasks { - let (id, status) = t.await.unwrap(); - assert_eq!( - status, - StatusCode::CREATED, - "POST {id} must succeed under concurrent distinct-id POSTs" - ); - succeeded.push(id); - } - - // Final registry has 1 (alpha) + N (graph-0..N-1) = N+1 graphs. - let resp = (*app) - .clone() - .oneshot( - Request::builder() - .method(Method::GET) - .uri("/graphs") - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(resp.status(), StatusCode::OK); - let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap(); - let payload: Value = serde_json::from_slice(&body).unwrap(); - let graph_count = payload["graphs"].as_array().unwrap().len(); - assert_eq!(graph_count, N + 1); - } } diff --git a/docs/releases/v0.7.0.md b/docs/releases/v0.7.0.md index 11d924d..1b619e6 100644 --- a/docs/releases/v0.7.0.md +++ b/docs/releases/v0.7.0.md @@ -1,6 +1,8 @@ # Omnigraph v0.7.0 -Multi-graph server mode (MR-668). One `omnigraph-server` process can now serve 1–10 graphs concurrently behind cluster routes (`/graphs/{graph_id}/...`), with per-graph Cedar policy, runtime graph creation via `POST /graphs`, and CLI parity (`omnigraph graphs list/create`). +Multi-graph server mode (MR-668). One `omnigraph-server` process can now serve 1–10 graphs concurrently behind cluster routes (`/graphs/{graph_id}/...`), with per-graph and server-level Cedar policy, read-only `GET /graphs` enumeration, and CLI parity (`omnigraph graphs list`). + +Runtime add/remove (`POST /graphs`, `DELETE /graphs/{id}`, `omnigraph graphs create`) is **not** in v0.7.0. Operators add or remove graphs by editing `omnigraph.yaml` and restarting. The first cut of `POST /graphs` shipped behind an atomic-YAML-rewrite design that we pulled before release once its concurrency guarantees were challenged (flock-on-renamed-inode race, duplicate-check outside the critical section, and an init-cleanup path that could destroy an existing graph's schema on re-init). The correct fix is a Lance-style cluster catalog (reserve → init → publish with recovery sidecars); that work is deferred. ## Breaking Changes @@ -13,25 +15,11 @@ Multi-graph server mode (MR-668). One `omnigraph-server` process can now serve 1 ## New - **Multi-graph mode**. Invoke with `omnigraph-server --config omnigraph.yaml` where the YAML has a non-empty `graphs:` map and no single-mode selector (no `server.graph`, no CLI `` or `--target`). At startup the server opens every configured graph in parallel (bounded concurrency, fail-fast). -- **`POST /graphs`**. Runtime graph creation. Request body: - ```json - { - "graph_id": "beta", - "uri": "/data/beta.omni", - "schema": { "source": "" }, - "policy": { "file": "./policies/beta.yaml" } - } - ``` - `schema` and `policy` are nested objects — leaves room for future fields without breaking the shape. (Asymmetric with the existing `POST /schema/apply`, which still uses flat `schema_source: String`. A follow-up release may migrate it.) Body limit is 32 MiB. - - The server runs `Omnigraph::init` at the supplied URI, atomically rewrites `omnigraph.yaml` under an exclusive `fcntl::flock` with SHA-256 drift detection, then publishes the handle in the in-memory registry. Returns 201 on success; 409 on duplicate `graph_id` or URI; 503 on YAML drift (operator hand-edited the file between server start and the rewrite). - **`GET /graphs`**. Lists every registered graph, sorted alphabetically by `graph_id`. Auth-required when bearer tokens are configured; Cedar-gated by `PolicyAction::GraphList` against `Omnigraph::Server::"root"`. Returns 405 in single mode. -- **CLI `omnigraph graphs list/create`**. Mirrors the HTTP surface. Reject local URI targets with a clear message — these subcommands are for remote multi-graph servers only. -- **Per-graph Cedar policy**. Each entry in the `graphs:` map can carry a `policy.file` path. Loaded at startup or attached at `POST` time. Cedar's `Omnigraph::Graph::""` resource is per-graph; the new `Omnigraph::Server::"root"` resource governs server-level actions. -- **Cedar action vocabulary**: `graph_create` and `graph_list` (server-scoped). `graph_delete` is reserved but not shipped — see "Deferred." -- **YAML drift detection**. Server hashes `omnigraph.yaml` at startup. `POST /graphs` re-hashes the on-disk file under the flock before rewriting; if the hash doesn't match the baseline, the rewrite refuses with 503 to avoid clobbering operator hand-edits. -- **`Omnigraph::init` error-path cleanup**. A failed init now best-effort-deletes the schema artifacts (`_schema.pg`, `_schema.ir.json`, `__schema_state.json`). Lance per-type directories created by `GraphCoordinator::init` may still orphan — full recursive cleanup needs a `delete_prefix` substrate primitive, deferred along with `DELETE /graphs/{id}`. -- **`omnigraph-policy` is now a published workspace crate.** The published-crates set is `omnigraph-compiler`, `omnigraph-policy`, `omnigraph-engine`, `omnigraph-server`, `omnigraph-cli`. +- **CLI `omnigraph graphs list`**. Mirrors the HTTP surface. Rejects local URI targets with a clear message — for remote multi-graph servers only. +- **Per-graph Cedar policy**. Each entry in the `graphs:` map can carry a `policy.file` path, loaded at startup. Cedar's `Omnigraph::Graph::""` resource is per-graph; the new `Omnigraph::Server::"root"` resource governs server-level actions. +- **Server-level Cedar policy**. `server.policy.file` in the config governs the `graph_list` action on `Omnigraph::Server::"root"`. Required to expose `GET /graphs` once bearer tokens are configured (MR-723 default-deny otherwise rejects `graph_list` as non-`read`). +- **Cedar action vocabulary**: `graph_list` (server-scoped). Runtime `graph_create` / `graph_delete` are reserved but not shipped — see "Deferred." ## Configuration @@ -41,7 +29,7 @@ Multi-graph server mode (MR-668). One `omnigraph-server` process can now serve 1 server: bind: 0.0.0.0:8080 policy: - file: ./server-policy.yaml # server-level Cedar (graph_create, graph_list) + file: ./server-policy.yaml # server-level Cedar (graph_list) graphs: alpha: @@ -55,8 +43,9 @@ graphs: ## Deferred -- **`DELETE /graphs/{id}`**. Cut from v0.7.0 scope to bound complexity (no `delete_prefix` substrate, no tombstones). Operators remove graphs by stopping the server, editing `omnigraph.yaml`, then restarting. -- **`StorageAdapter::delete_prefix`**. The substrate primitive that DELETE would need. Will land alongside DELETE in a future release. +- **`POST /graphs` runtime graph creation** and **CLI `omnigraph graphs create`**. Pulled before release after the YAML-rewrite design's correctness story didn't survive review. A future release will add a managed cluster catalog (Lance-backed reserve → init → publish with recovery sidecars) and re-expose runtime creation on top of it. Until then, operators add graphs by editing `omnigraph.yaml` and restarting. +- **`DELETE /graphs/{id}`**. Never shipped in v0.7.0; deferred with the same cluster-catalog work. +- **`StorageAdapter::delete_prefix`**. The substrate primitive a managed catalog would need. Will land alongside runtime mutation. - **`X-Actor-Id` service delegation forwarding**. Needs durable both-actor audit on `_graph_commits.lance` — out of scope. - **Hot policy reload**. Restart is cheap at N≤10 graphs. @@ -65,7 +54,6 @@ graphs: - **Existing single-graph deployments upgrade with zero changes.** `omnigraph-server ` with v0.6.0 config keeps working identically. - **Multi-graph adoption is opt-in.** Add a `graphs:` map to `omnigraph.yaml` (and remove `server.graph`) to switch a deployment to multi mode. - **Cluster routes are breaking for client SDKs targeting multi mode.** Generated clients from previous v0.6.0 OpenAPI specs will hit 404 on flat paths against a multi-mode server. Regenerate against the v0.7.0 `openapi.json`. -- **`fs2 = "0.4"`** is a new dependency for the file locking that powers the atomic YAML rewrite. POSIX-only. Linux / macOS deployment supported; Windows is out of scope. - **Operator-supplied policy.yaml files don't change.** The Cedar `Omnigraph::Graph` and `Omnigraph::Server` entities are internally generated by `compile_policy_source` — operator YAML only references actions and groups. ## Migration: single → multi @@ -85,7 +73,7 @@ policy: # After (v0.7.0 multi-mode — drop `server.graph` and the top-level `policy`) server: policy: - file: ./server-policy.yaml # NEW: governs POST/GET /graphs + file: ./server-policy.yaml # NEW: governs GET /graphs graphs: my-graph: uri: /var/lib/omnigraph/my-graph @@ -95,15 +83,12 @@ graphs: Same `omnigraph.yaml` file; restart the server. Clients targeting the old flat routes (`/snapshot`, `/read`, …) must update to `/graphs/my-graph/snapshot`, etc. +To add a new graph after rollout: stop the server, append a new `graphs.` entry, restart. + ## Test coverage -v0.7.0 ships ~280 new tests covering MR-668 specifically: - -- `GraphId` newtype validation, registry race tests (PR 3), init failpoints (PR 2a). +- `GraphId` newtype validation, registry race tests (PR 3), init failpoints (PR 2a — still reachable from `omnigraph init` CLI). - Mode-inference four-rule matrix (PR 5), parallel multi-graph startup, cluster routing. - Cedar `Server` resource refactor, backwards-compat for graph-only policies. -- `POST /graphs` happy path + duplicate graph_id + duplicate URI + YAML drift detection + 405-in-single-mode. -- Composite lifecycle: POST a graph, query it via cluster route, reload config from disk, confirm persistence. -- Per-graph Cedar policy enforced for a POST-created graph (engine-layer enforcement is re-applied via `Omnigraph::with_policy`). -- Concurrent distinct-id POSTs serialize correctly through the flock without spurious drift errors. +- `GET /graphs` enumeration, 405-in-single-mode. - MR-731 spoof regression test stays green across the entire refactor. diff --git a/docs/user/cli.md b/docs/user/cli.md index da0127a..35de149 100644 --- a/docs/user/cli.md +++ b/docs/user/cli.md @@ -46,26 +46,17 @@ and configure the matching `bearer_token_env` in `omnigraph.yaml`. ## Multi-graph servers (v0.7.0+) -Against a multi-graph server (started with `--config omnigraph.yaml` referencing a non-empty `graphs:` map), use `omnigraph graphs` to enumerate and create graphs: +Against a multi-graph server (started with `--config omnigraph.yaml` referencing a non-empty `graphs:` map), use `omnigraph graphs list` to enumerate the registered graphs: ```bash -# List -omnigraph graphs list --target http://server.example.com --json - -# Create -omnigraph graphs create \ - --target http://server.example.com \ - --graph-id beta \ - --graph-uri /data/beta.omni \ - --schema schema.pg \ - --policy-file ./policies/beta.yaml # optional +omnigraph graphs list --uri http://server.example.com --json ``` -The CLI reads `--schema` from the local disk and inlines the contents as `schema.source` in the request body. Both subcommands reject local URI targets — they're for remote multi-graph servers only. +`list` rejects local URI targets — it's for remote multi-graph servers only. -`omnigraph graphs delete` is **not** in v0.7.0. To remove a graph, stop the server, edit `omnigraph.yaml`, restart. +Runtime add/remove is **not** in v0.7.0. To add a graph, stop the server, add a `graphs.` entry to `omnigraph.yaml`, then restart. To remove, stop the server, delete the entry, restart. -Per-graph URLs: once a graph exists, hit its cluster route from any subcommand by pointing `--uri` at it: +Per-graph URLs: hit a graph's cluster route from any subcommand by pointing `--uri` at it: ```bash omnigraph read --uri http://server.example.com/graphs/beta --query ./q.gq ... diff --git a/docs/user/policy.md b/docs/user/policy.md index 946092c..7628f53 100644 --- a/docs/user/policy.md +++ b/docs/user/policy.md @@ -15,12 +15,11 @@ Per-graph actions (bind to `Omnigraph::Graph::""`): 7. `branch_merge` 8. `admin` — reserved for policy-management surfaces (hot reload, audit log, approvals). No call site today; see MR-724 for the reservation rationale. -Server-scoped actions (v0.7.0+; bind to `Omnigraph::Server::"root"`): +Server-scoped action (v0.7.0+; binds to `Omnigraph::Server::"root"`): -9. `graph_create` — `POST /graphs` runtime graph creation (multi-graph mode) -10. `graph_list` — `GET /graphs` registry enumeration (multi-graph mode) +9. `graph_list` — `GET /graphs` registry enumeration (multi-graph mode) -Server-scoped actions cannot use `branch_scope` or `target_branch_scope` — they operate on the registry, not on a graph's branches. A rule cannot mix server-scoped and per-graph actions; split into separate rules. (`graph_delete` is reserved but not shipped in v0.7.0.) +Server-scoped actions cannot use `branch_scope` or `target_branch_scope` — they operate on the registry, not on a graph's branches. A rule cannot mix server-scoped and per-graph actions; split into separate rules. (Runtime `graph_create` / `graph_delete` are reserved but not shipped in v0.7.0; operators add/remove graphs by editing `omnigraph.yaml` and restarting.) ## Scope kinds @@ -35,7 +34,7 @@ In multi mode (`omnigraph.yaml` with a non-empty `graphs:` map), policy files at ```yaml server: policy: - file: ./server-policy.yaml # server-level: graph_create, graph_list + file: ./server-policy.yaml # server-level: graph_list graphs: alpha: @@ -47,7 +46,7 @@ graphs: # no per-graph policy → no engine-layer Cedar enforcement on beta ``` -Each graph's HTTP request flows through its own per-graph policy. Management endpoints (`/graphs`) flow through the server-level policy. When `server.policy.file` is unset and bearer tokens are configured, `GET /graphs` falls through to MR-723 default-deny (only `read`-equivalent actions allowed for authenticated actors — and `graph_list` is not `read`) → 403. So the operator must explicitly authorize via `server-policy.yaml` to expose `/graphs`. +Each graph's HTTP request flows through its own per-graph policy. The management endpoint (`GET /graphs`) flows through the server-level policy. When `server.policy.file` is unset and bearer tokens are configured, `GET /graphs` falls through to MR-723 default-deny (only `read`-equivalent actions allowed for authenticated actors — and `graph_list` is not `read`) → 403. So the operator must explicitly authorize via `server-policy.yaml` to expose `/graphs`. Example server-level policy: @@ -56,10 +55,10 @@ version: 1 groups: admins: [act-andrew] rules: - - id: admins-can-create-and-list-graphs + - id: admins-can-list-graphs allow: actors: { group: admins } - actions: [graph_create, graph_list] + actions: [graph_list] ``` ## Configuration @@ -75,7 +74,7 @@ cli: actor: act-andrew # default actor for CLI direct-engine writes ``` -Each rule must use exactly one of `branch_scope` or `target_branch_scope`. +Each per-graph rule must use exactly one of `branch_scope` or `target_branch_scope`. Server-scoped rules (`graph_list`) take neither — they have no branch context. `cli.actor` is the default actor identity for CLI direct-engine writes when `policy.file` is configured. Override per-invocation with `--as diff --git a/docs/user/server.md b/docs/user/server.md index a9a86f5..144604a 100644 --- a/docs/user/server.md +++ b/docs/user/server.md @@ -47,34 +47,18 @@ Server-level management endpoints (v0.7.0+): | Method | Path | Auth | Action | Handler | |---|---|---|---|---| | GET | `/graphs` | bearer + `graph_list` on `Server::"root"` | list registered graphs | `server_graphs_list` (405 in single mode) | -| POST | `/graphs` | bearer + `graph_create` on `Server::"root"` | create new graph at runtime | `server_graphs_create` (405 in single mode, 32 MB body limit) | -`DELETE /graphs/{id}` is **not** in v0.7.0. Operators remove graphs by stopping the server, editing `omnigraph.yaml`, then restarting. +## Adding and removing graphs (multi mode) -## `omnigraph.yaml` ownership (multi mode) +Runtime add/remove via API is **not** exposed in v0.7.0 — neither +`POST /graphs` nor `DELETE /graphs/{id}` is implemented. Operators add +or remove graphs by stopping the server, editing the `graphs:` map in +`omnigraph.yaml`, then restarting. The server treats `omnigraph.yaml` +as operator-owned configuration and never writes it. -The server owns `omnigraph.yaml` while running. `POST /graphs` rewrites the file atomically under an exclusive `fcntl::flock` with SHA-256 drift detection: - -- The server hashes the file at startup. `POST /graphs` re-hashes under the flock before rewriting. If the hash doesn't match (operator hand-edited), the rewrite refuses with 503. -- Comments and blank-line structure are **not** preserved across server-side rewrites — the file is regenerated via `serde_yaml::to_string`. -- Operators must not edit the file while the server is running. To make offline changes: stop the server, edit, restart. - -In **single mode** the server never writes `omnigraph.yaml`. - -## `POST /graphs` body shape - -```json -{ - "graph_id": "alpha", - "uri": "s3://tenant-bucket/alpha", - "schema": { "source": "" }, - "policy": { "file": "./policies/alpha.yaml" } -} -``` - -- `schema` and `policy` are nested — leaves room for future fields without breaking the shape. -- `policy` is optional; without it, no per-graph Cedar enforcement. -- Status codes: 201 Created · 400 invalid body · 401 missing bearer · 403 Cedar denied · 405 single mode · 409 duplicate `graph_id` or `uri` · 413 body >32 MiB · 500 init or rewrite failure · 503 YAML drift. +A future release may introduce a managed registry (Lance-backed, +catalog-style: reserve → init → publish with recovery sidecars) and +re-expose runtime mutation on top of it. ## Streaming diff --git a/openapi.json b/openapi.json index 5d326fe..f5bc244 100644 --- a/openapi.json +++ b/openapi.json @@ -640,121 +640,6 @@ "bearer_token": [] } ] - }, - "post": { - "tags": [ - "management" - ], - "summary": "Create a new graph at runtime (MR-668 PR 7).", - "description": "Multi-graph mode only. Operators add a graph to the registry\nwithout restarting the server. The server `Omnigraph::init`s the\nnew graph at `req.uri`, atomically rewrites `omnigraph.yaml` to\ninclude the new entry, then publishes the handle in the registry.\n\nCedar-gated by `PolicyAction::GraphCreate` against\n`Omnigraph::Server::\"root\"` (the same server-level policy as\n`GET /graphs`).\n\nFailure modes:\n* Init fails → orphan storage files at `req.uri` (PR 2a cleans up\n schema files but not Lance datasets; operator removes manually).\n* Rewrite fails (`fs2::flock` IO error) → orphan storage; YAML\n unchanged.\n* YAML drift (operator edited the file) → 503; YAML and storage\n both unchanged.\n* Duplicate `graph_id` or `uri` → 409; storage already in use.", - "operationId": "createGraph", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/GraphCreateRequest" - } - } - }, - "required": true - }, - "responses": { - "201": { - "description": "Graph created", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/GraphCreateResponse" - } - } - } - }, - "400": { - "description": "Invalid request body (graph_id, schema, policy file)", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "401": { - "description": "Unauthorized", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "403": { - "description": "Forbidden", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "405": { - "description": "Method not allowed (single-graph mode)", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "409": { - "description": "graph_id or uri already registered", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "413": { - "description": "Request body too large (>32 MiB)", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "500": { - "description": "Init failure or YAML rewrite failure", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - }, - "503": { - "description": "omnigraph.yaml drift detected (operator edited the file)", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ErrorOutput" - } - } - } - } - }, - "security": [ - { - "bearer_token": [] - } - ] } }, "/healthz": { @@ -1440,56 +1325,6 @@ } } }, - "GraphCreateRequest": { - "type": "object", - "description": "Request body for `POST /graphs` (MR-668 PR 7).\n\nBody shape:\n```json\n{\n \"graph_id\": \"alpha\",\n \"uri\": \"/path/to/alpha.omni\",\n \"schema\": { \"source\": \"\" },\n \"policy\": { \"file\": \"./policies/alpha.yaml\" }\n}\n```\n\n32 MiB body limit (matches `INGEST_REQUEST_BODY_LIMIT_BYTES`).", - "required": [ - "graph_id", - "uri", - "schema" - ], - "properties": { - "graph_id": { - "type": "string", - "description": "New graph's id. Must satisfy `^[a-zA-Z0-9-]{1,64}$`, not start with\n`_`, and not be a reserved name. See `GraphId::try_from`." - }, - "policy": { - "oneOf": [ - { - "type": "null" - }, - { - "$ref": "#/components/schemas/GraphPolicySpec", - "description": "Per-graph Cedar policy. Optional — `None` means the graph has\nno per-graph policy enforcement (HTTP auth still applies if\nconfigured)." - } - ] - }, - "schema": { - "$ref": "#/components/schemas/GraphSchemaSpec", - "description": "Inline schema (`{ source }`). Required." - }, - "uri": { - "type": "string", - "description": "Storage URI (local path or `s3://...`). Must NOT already be in\nuse by another registered graph. Server `Omnigraph::init`s the\ngraph at this URI." - } - } - }, - "GraphCreateResponse": { - "type": "object", - "description": "Response from `POST /graphs` on success (201 Created).", - "required": [ - "graph_id", - "uri" - ], - "properties": { - "graph_id": { - "type": "string" - }, - "uri": { - "type": "string" - } - } - }, "GraphInfo": { "type": "object", "description": "One entry in the response from `GET /graphs`. Cluster operators\nconsume this list to discover which graphs the server is currently\nserving. The shape is intentionally minimal — `graph_id` and `uri`\nare the only fields a routing client needs.", @@ -1521,33 +1356,6 @@ } } }, - "GraphPolicySpec": { - "type": "object", - "description": "Per-graph policy specification in `POST /graphs`. Mirrors the\n`policy: { file }` shape in `omnigraph.yaml`'s `graphs..policy`\nsection.", - "properties": { - "file": { - "type": [ - "string", - "null" - ], - "description": "Path to the per-graph Cedar policy file, server-side.\nMust be readable by the server process at request time.\nPath is relative to the server's working directory (NOT to the\n`omnigraph.yaml`'s `base_dir`) — caller-supplied paths are\ntrusted as-is." - } - } - }, - "GraphSchemaSpec": { - "type": "object", - "description": "Schema specification for a new graph in `POST /graphs`. Nested\nper MR-668 decision 7 — leaves room for future fields without\nbreaking the request shape. Mirrors the `policy: { file }` nesting\npattern.\n\nToday only `source` (inline `.pg` text) is supported. Future fields\nmight include `schema.allow_data_loss`, `schema.version`, etc.\n\n**Asymmetric with `SchemaApplyRequest`**: `POST /schema/apply` still\nuses a flat `schema_source: String` for backwards compatibility.\nA follow-up release may migrate that too.", - "required": [ - "source" - ], - "properties": { - "source": { - "type": "string", - "description": "Inline `.pg` schema source.", - "example": "node Person {\n name: String @key\n age: I32?\n}" - } - } - }, "HealthOutput": { "type": "object", "required": [