mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(datatables): hold the parent's settings while a fork points at its data tables
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ee176e24d5
commit
8a7f364cfe
@@ -1071,6 +1071,60 @@ async fn an_alias_saved_elsewhere_waits_for_roles_going_on_for_its_database(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn a_fork_waits_for_a_rename_of_the_data_table_it_keeps(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
// A rename in flight holds the parent's settings row and moves only the pointers it can see; a
|
||||
// fork being created is invisible to it, so the fork has to read the name the rename commits.
|
||||
let mut renaming = db.begin().await?;
|
||||
sqlx::query(
|
||||
"SELECT 1 FROM workspace_settings WHERE workspace_id = 'test-workspace' FOR UPDATE",
|
||||
)
|
||||
.execute(&mut *renaming)
|
||||
.await?;
|
||||
sqlx::query(
|
||||
"UPDATE workspace_settings SET datatable = jsonb_set(datatable #- '{datatables,main}',
|
||||
'{datatables,renamed}', datatable->'datatables'->'main')
|
||||
WHERE workspace_id = 'test-workspace'",
|
||||
)
|
||||
.execute(&mut *renaming)
|
||||
.await?;
|
||||
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let url = format!(
|
||||
"http://localhost:{}/api/w/test-workspace/workspaces/create_fork",
|
||||
server.addr.port()
|
||||
);
|
||||
let fork = tokio::spawn(
|
||||
authed(client().post(&url), "SECRET_TOKEN")
|
||||
.json(&json!({ "id": "wm-fork-race", "name": "race", "color": "#0000ff" }))
|
||||
.send(),
|
||||
);
|
||||
tokio::time::sleep(std::time::Duration::from_millis(500)).await;
|
||||
assert!(
|
||||
!fork.is_finished(),
|
||||
"the fork copied the parent's data tables while a rename held them"
|
||||
);
|
||||
renaming.commit().await?;
|
||||
|
||||
let resp = fork.await??;
|
||||
assert!(resp.status().is_success(), "{}", resp.text().await?);
|
||||
let datatables: Option<Value> = sqlx::query_scalar(
|
||||
"SELECT datatable->'datatables' FROM workspace_settings WHERE workspace_id = 'wm-fork-race'",
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
let datatables = datatables.unwrap();
|
||||
assert_eq!(
|
||||
datatables["renamed"]["reference"],
|
||||
json!({ "workspace_id": "test-workspace", "datatable": "renamed" }),
|
||||
"{datatables}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(all(feature = "private", feature = "enterprise")))]
|
||||
const ENTERPRISE_REFUSAL: &str = "Data table roles are a Windmill Enterprise Edition feature";
|
||||
|
||||
|
||||
@@ -8549,6 +8549,14 @@ async fn create_workspace_fork(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// The pointers this fork writes to the parent's data tables stay invisible until it commits, so
|
||||
// a rename of one of them cannot carry them. Holding the parent's settings row makes such a
|
||||
// rename wait for this commit, and makes the copy below read one that committed first.
|
||||
sqlx::query("SELECT 1 FROM workspace_settings WHERE workspace_id = $1 FOR SHARE")
|
||||
.bind(&parent_workspace_id)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Clone all data from the parent workspace using Rust implementation
|
||||
if let Err(e) =
|
||||
clone_workspace_data(&mut tx, &db, &parent_workspace_id, &forked_id, &authed).await
|
||||
|
||||
@@ -233,6 +233,9 @@ pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Res
|
||||
|
||||
/// Every instance database the registry knows about. Role provisioning has to reach all of them:
|
||||
/// a role that cannot `CONNECT` to a database is refused by Postgres before any grant matters.
|
||||
///
|
||||
/// Authorization: checks nothing, and names every instance database across all workspaces. Callers
|
||||
/// MUST be superadmin-gated or keep the names server-side; never return them to a workspace caller.
|
||||
pub async fn registered_instance_databases(db: &DB) -> Result<Vec<String>> {
|
||||
crate::datatable_roles_oss::registered_instance_databases(db).await
|
||||
}
|
||||
|
||||
@@ -1967,6 +1967,10 @@ pub fn strip_datatable_permissions(
|
||||
/// As [`parse_datatable_ref`], except that an entry whose stored name itself contains `?` — which
|
||||
/// names could before they were restricted — resolves by that exact name, without a role. It is
|
||||
/// looked up first, so `sales?role=x` never reaches a different entry than the one stored so.
|
||||
///
|
||||
/// Authorization: checks nothing, and its answer reveals whether `w_id` stores that exact name.
|
||||
/// Callers MUST already act for `w_id` — a job of it, or a caller authenticated into it — and
|
||||
/// MUST still pass the name to [`get_datatable_resource_from_db`] or an admin-access check.
|
||||
pub async fn parse_datatable_ref_for(
|
||||
db: &DB,
|
||||
w_id: &str,
|
||||
|
||||
@@ -377,7 +377,10 @@ pub async fn get_raw_postgres_connection(
|
||||
/// A replication stream reads every row of every table whatever the data table's roles grant, so
|
||||
/// the two don't mix: a data table under roles takes no triggers or captures, and roles cannot be
|
||||
/// turned on while one is enabled on it.
|
||||
pub async fn ensure_not_under_roles(
|
||||
///
|
||||
/// Authorization: checks nothing, and its refusal says whether `w_id`'s data table is under roles.
|
||||
/// Callers MUST have established that the caller may manage triggers in `w_id` first.
|
||||
pub(crate) async fn ensure_not_under_roles(
|
||||
db: &DB,
|
||||
w_id: &str,
|
||||
postgres_resource_path: &str,
|
||||
|
||||
Reference in New Issue
Block a user