diff --git a/backend/parsers/windmill-parser/src/asset_parser.rs b/backend/parsers/windmill-parser/src/asset_parser.rs index da3ef9b700..30c6047255 100644 --- a/backend/parsers/windmill-parser/src/asset_parser.rs +++ b/backend/parsers/windmill-parser/src/asset_parser.rs @@ -1752,17 +1752,6 @@ mod pipeline_annotation_tests { // annotation is hand-written, and the warehouses fold case in opposite // directions. A regression here is invisible: both nodes still render, they // just stop being the same node and the cross-boundary cascade never fires. - /// The shape both halves of the deploy check against: a subscription and a - /// write that disagreed on it would refuse and accept the same string. - #[test] - fn a_whole_relation_is_three_non_empty_segments() { - assert!(is_full_relation_path("main/analytics/orders")); - assert!(is_full_relation_path("main/archive.sales/orders")); - for partial in ["main", "main/analytics", "main/analytics/orders/x", "", "main//orders"] { - assert!(!is_full_relation_path(partial), "{partial} is not a relation"); - } - } - #[test] fn table_paths_from_every_spelling_canonicalize_to_one_key() { let canonical = Some((AssetKind::Dbt, Cow::Owned("main/analytics/orders".into()))); @@ -1787,6 +1776,17 @@ mod pipeline_annotation_tests { } } + /// The shape both halves of the deploy check against: a subscription and a + /// write that disagreed on it would refuse and accept the same string. + #[test] + fn a_whole_relation_is_three_non_empty_segments() { + assert!(is_full_relation_path("main/analytics/orders")); + assert!(is_full_relation_path("main/archive.sales/orders")); + for partial in ["main", "main/analytics", "main/analytics/orders/x", "", "main//orders"] { + assert!(!is_full_relation_path(partial), "{partial} is not a relation"); + } + } + // A relation that overrode its database carries `.` in // one segment, and each half can be quoted independently. Stripping only // the outer pair leaves a key the manifest ingest never produces, so the diff --git a/backend/windmill-api-integration-tests/tests/dbt_materialize_target.rs b/backend/windmill-api-integration-tests/tests/dbt_materialize_target.rs index ecc9add82b..ef252f0ac9 100644 --- a/backend/windmill-api-integration-tests/tests/dbt_materialize_target.rs +++ b/backend/windmill-api-integration-tests/tests/dbt_materialize_target.rs @@ -85,16 +85,30 @@ async fn test_dbt_materialize_target_deploy_contract(db: Pool) -> anyh .await? .contains("`// data_test` is not supported")); - // Every producer spells a whole relation, so a partial one is an edge nothing - // can ever wake. - let resp = deploy( - port, - "u/test-user/partial_sub", - "// on dbt://main/analytics\nexport async function main() {}", - ) - .await; - assert_eq!(resp.status(), 400); - assert!(resp.text().await?.contains("not a whole warehouse relation")); + // Both halves are held to the same relation: every producer is a whole + // `//` under a configured warehouse, so a + // subscription to anything else names something nothing can ever write. + for (path, ref_, expected) in [ + ( + "u/test-user/partial_sub", + "dbt://main/analytics", + "not a whole warehouse relation", + ), + ( + "u/test-user/unknown_wh_sub", + "dbt://nope/analytics/orders", + "does not configure", + ), + ] { + let resp = deploy( + port, + path, + &format!("// on {ref_}\nexport async function main() {{}}"), + ) + .await; + assert_eq!(resp.status(), 400); + assert!(resp.text().await?.contains(expected)); + } // Any language may declare the write — the DuckLake write engine is DuckDB's, // this declaration is not — and the target is canonicalized on the way into diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 6c1a516fe0..6850cec8fb 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -1081,6 +1081,39 @@ fn lock_hash_entry(path: &str, lock: &str) -> [(String, i64); 1] { [(path.to_string(), hash_script(lock))] } +/// The `dbt://` relation both halves of a deploy have to agree on: a whole +/// `//`, under a warehouse this workspace configures. +/// +/// Every producer is held to exactly this — a `// materialize` target here, a +/// descriptor's `profile.warehouse` in the worker — so a subscription to anything +/// else names a relation nothing can ever write. No later deploy fixes that and +/// no dormant-edge warning reports it, since the warning fires on a dbt project's +/// ingest and no project can claim a relation under a warehouse that isn't there. +/// Asking here rather than at each site is what keeps the two from drifting into +/// refusing and accepting the same string. +async fn validate_dbt_relation( + db: &sqlx::Pool, + w_id: &str, + relation: &str, + what: &str, +) -> Result<()> { + if !windmill_parser::asset_parser::is_full_relation_path(relation) { + return Err(Error::BadRequest(format!( + "{what} `dbt://{relation}` is not a whole warehouse relation \ + (`dbt:////`), so nothing can produce it." + ))); + } + let warehouse = relation.split('/').next().unwrap_or_default(); + windmill_common::workspaces::dbt_warehouse_exists(db, w_id, warehouse) + .await + .map_err(|e| { + Error::BadRequest(format!( + "{what} `dbt://{relation}` names a warehouse this workspace does not \ + configure: {e}" + )) + }) +} + async fn create_script_internal<'c>( mut ns: NewScript, w_id: String, @@ -1638,26 +1671,8 @@ async fn create_script_internal<'c>( .to_string(), )); } - if !windmill_parser::asset_parser::is_full_relation_path(&m.target_path) { - return Err(Error::BadRequest(format!( - "`// materialize` needs a full warehouse relation in the target: \ - `dbt:////` (got `dbt://{}`).", - m.target_path - ))); - } - let warehouse = m.target_path.split('/').next().unwrap_or_default(); - // The warehouse segment IS the identity a dbt model reading this - // relation keys on, so a name no warehouse answers to is not a - // namespace — it strands this write on a node nothing reaches. - // Same resolution a dbt descriptor's `profile.warehouse` gets. - windmill_common::workspaces::dbt_warehouse_exists(&db, &w_id, warehouse) - .await - .map_err(|e| { - Error::BadRequest(format!( - "`// materialize dbt://{warehouse}/…` names a warehouse this \ - workspace does not configure: {e}" - )) - })?; + validate_dbt_relation(&db, &w_id, &m.target_path, "`// materialize` target") + .await?; } _ => { return Err(Error::BadRequest( @@ -2467,15 +2482,7 @@ async fn create_script_internal<'c>( and a project is run on its schedule, not woken by an asset cascade." ))); } - // Every producer spells a whole relation, so a partial one is an edge - // nothing can ever wake — refused on the same terms as the dbt-only - // case rather than persisted. - if !windmill_parser::asset_parser::is_full_relation_path(relation) { - return Err(Error::BadRequest(format!( - "`{trigger_ref}` is not a whole warehouse relation \ - (`dbt:////`), so nothing can produce it." - ))); - } + validate_dbt_relation(&db, &w_id, relation, "subscription target").await?; // Both paths under a rename: the old one's committed write row is // still there and this transaction is about to remove it. let deploying_paths = match p_path_opt.as_deref().filter(|old| *old != ns.path) { diff --git a/docs/dbt-runtime.md b/docs/dbt-runtime.md index beb37fd5a2..b7ec709e04 100644 --- a/docs/dbt-runtime.md +++ b/docs/dbt-runtime.md @@ -714,8 +714,11 @@ fan-out reads the deploy-time `asset` rows) but records no row, so the relation shows no last writer. Fixing it is one change for all of those annotations, not this one. -A `# on dbt://` subscription is therefore refused at deploy in exactly -one shape: when every script that writes that relation is a dbt one. Nothing +A `# on dbt://` subscription is held to the same relation a producer +is — a whole `//` under a configured warehouse, checked +by the validator the `// materialize` target goes through, since two spellings of +that rule would refuse and accept the same string. Beyond that it is refused in +exactly one shape: when every script that writes that relation is a dbt one. Nothing produces it yet is NOT that shape — a subscriber may be deployed before its producer, as for every other asset kind, and refusing there would break deploy-order-independent syncs. A dbt script may neither subscribe nor declare a