mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-20 16:03:21 +00:00
feat: isolate fork object storage under a per-fork prefix (#11829)
* feat: route the python SDK's S3 client settings through the S3 proxy Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: drop the boto3 config key and expose the proxied bucket to polars Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: proxy S3 settings only for server-signed tokens and keep tls verify Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: isolate fork object storage under a per-fork prefix with read-through Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: guard fork storage isolation against traversal, shared lakes and orphaning Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: serialize fork purges with same-id creation and guard shared middle forks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: drop fork storage mappings on every replica and lock fork deletion against child creation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref to 455681e55eb7cdfcfdb7cd79cbae75b333e07943 This commit updates the EE repository reference after PR #893 was merged in windmill-ee-private. Previous ee-repo-ref: 93f26e3cf816e4eb97b7d4dbbb5fc97fdf9821af New ee-repo-ref: 455681e55eb7cdfcfdb7cd79cbae75b333e07943 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
parent
e0138a7374
commit
9375805fdf
@@ -515,6 +515,11 @@ struct CreateWorkspaceFork {
|
||||
/// fork namespace + read-defer.
|
||||
#[serde(default)]
|
||||
shared_ducklakes: Vec<String>,
|
||||
/// Workspace storages the fork reads and writes at the parent's keys (`_default_` for the
|
||||
/// primary one). Every other storage is isolated: the fork writes under its own prefix
|
||||
/// and reads through to the parent's objects.
|
||||
#[serde(default)]
|
||||
shared_storages: Vec<String>,
|
||||
/// Create the fork as a persistent dev workspace: the id is not required to carry the
|
||||
/// `wm-fork-` prefix, and at most one dev workspace may exist per parent.
|
||||
#[serde(default)]
|
||||
@@ -2131,6 +2136,38 @@ async fn edit_dbt_warehouses(
|
||||
Ok("Updated the workspace's dbt warehouses".to_string())
|
||||
}
|
||||
|
||||
/// A fork's isolated storage is purged at deletion from the storage recorded at creation: under
|
||||
/// another location, its objects would outlive the fork untracked, and a same-id fork would
|
||||
/// read them.
|
||||
async fn reject_isolated_storage_repoint(
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
w_id: &str,
|
||||
new_lfs: &serde_json::Value,
|
||||
) -> Result<()> {
|
||||
let rows = sqlx::query!(
|
||||
"SELECT storage, storage_ref FROM fork_storage_namespace WHERE workspace_id = $1",
|
||||
w_id
|
||||
)
|
||||
.fetch_all(&mut **tx)
|
||||
.await?;
|
||||
for row in rows {
|
||||
let new_ref = windmill_common::fork_storage::lfs_entry(new_lfs, &row.storage)
|
||||
.and_then(windmill_common::workspaces::lfs_entry_storage_ref);
|
||||
if new_ref.is_some_and(|r| r != row.storage_ref) {
|
||||
let name = if row.storage.is_empty() {
|
||||
windmill_types::s3::DEFAULT_STORAGE
|
||||
} else {
|
||||
row.storage.as_str()
|
||||
};
|
||||
return Err(Error::BadRequest(format!(
|
||||
"storage `{name}` is isolated from the parent in this fork, so where it points \
|
||||
cannot be changed"
|
||||
)));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn edit_large_file_storage_config(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
@@ -2198,6 +2235,7 @@ async fn edit_large_file_storage_config(
|
||||
let serialized_lfs_config =
|
||||
serde_json::to_value::<LargeFileStorageWithSecondary>(lfs_config)
|
||||
.map_err(|err| Error::internal_err(err.to_string()))?;
|
||||
reject_isolated_storage_repoint(&mut tx, &w_id, &serialized_lfs_config).await?;
|
||||
|
||||
// A workspace whose AI session backups fell back to the instance store leaves it
|
||||
// here: the generation moves on, so nothing it left in any instance store is read
|
||||
@@ -9571,6 +9609,25 @@ async fn create_workspace_fork(
|
||||
i.msg
|
||||
);
|
||||
}
|
||||
// Unlike a dropped lake's files, a deleted fork's objects are live to a same-id fork: it
|
||||
// reads its own prefix first.
|
||||
let leftover_storages = crate::workspaces_extra::prepare_fork_storage_purges(
|
||||
&db,
|
||||
&nw.id,
|
||||
Some(&parent_workspace_id),
|
||||
)
|
||||
.await?;
|
||||
let storage_issues =
|
||||
crate::workspaces_extra::purge_fork_storages_unless_live(&db, &nw.id, leftover_storages)
|
||||
.await?;
|
||||
if !storage_issues.is_empty() {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"a previously deleted workspace with id '{}' left objects in its fork prefix that \
|
||||
could not be deleted: {}; retry once the storage is reachable again",
|
||||
nw.id,
|
||||
storage_issues.join("; ")
|
||||
)));
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "enterprise"))]
|
||||
_check_nb_of_workspaces(&db).await?;
|
||||
@@ -9964,6 +10021,97 @@ fn copy_requests(
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Record every storage of the fork's cloned settings as isolated, except `shared` ones.
|
||||
/// Isolation is a per-fork-creation choice, never inherited: a fork of a shared fork is
|
||||
/// isolated unless it asks otherwise.
|
||||
async fn isolate_fork_storages(
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
fork_id: &str,
|
||||
shared: &[String],
|
||||
shared_ducklakes: &[String],
|
||||
) -> Result<()> {
|
||||
use windmill_common::fork_storage::{lfs_entry, registry_storage_name};
|
||||
|
||||
let settings = sqlx::query!(
|
||||
"SELECT large_file_storage, ducklake FROM workspace_settings WHERE workspace_id = $1",
|
||||
fork_id
|
||||
)
|
||||
.fetch_optional(&mut **tx)
|
||||
.await?;
|
||||
let (lfs, ducklake) = settings
|
||||
.map(|s| (s.large_file_storage, s.ducklake))
|
||||
.unwrap_or_default();
|
||||
let lfs = lfs.filter(|v| v.get("type").is_some());
|
||||
|
||||
// A shared lake keeps the parent's data path: its files must land at the parent's keys, or
|
||||
// the parent's catalog would point at objects in the fork's prefix, purged with the fork.
|
||||
if lfs.is_some() {
|
||||
for lake in shared_ducklakes {
|
||||
let lake_storage = ducklake
|
||||
.as_ref()
|
||||
.and_then(|d| {
|
||||
d.get("ducklakes")?
|
||||
.get(lake)?
|
||||
.get("storage")?
|
||||
.get("storage")
|
||||
})
|
||||
.and_then(|s| s.as_str());
|
||||
let storage = registry_storage_name(lake_storage);
|
||||
if !shared
|
||||
.iter()
|
||||
.any(|s| registry_storage_name(Some(s)) == storage)
|
||||
{
|
||||
return Err(Error::BadRequest(format!(
|
||||
"ducklake `{lake}` is shared with the parent, so the storage holding its \
|
||||
data (`{}`) must be shared too",
|
||||
lake_storage.unwrap_or(windmill_types::s3::DEFAULT_STORAGE)
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
let mut storages: Vec<&str> = vec![];
|
||||
if let Some(lfs) = &lfs {
|
||||
storages.push("");
|
||||
if let Some(secondary) = lfs.get("secondary_storage").and_then(|s| s.as_object()) {
|
||||
storages.extend(secondary.keys().map(String::as_str));
|
||||
}
|
||||
}
|
||||
for name in shared {
|
||||
if !storages.contains(®istry_storage_name(Some(name))) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"cannot share storage `{name}` with the parent: no such storage in the workspace \
|
||||
settings"
|
||||
)));
|
||||
}
|
||||
}
|
||||
let Some(lfs) = &lfs else {
|
||||
return Ok(());
|
||||
};
|
||||
for storage in storages {
|
||||
if shared
|
||||
.iter()
|
||||
.any(|s| registry_storage_name(Some(s)) == storage)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
let Some(storage_ref) =
|
||||
lfs_entry(lfs, storage).and_then(windmill_common::workspaces::lfs_entry_storage_ref)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
sqlx::query!(
|
||||
"INSERT INTO fork_storage_namespace (workspace_id, storage, storage_ref)
|
||||
VALUES ($1, $2, $3)",
|
||||
fork_id,
|
||||
storage,
|
||||
storage_ref
|
||||
)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Write the fork, with its data tables pointed at `copies`.
|
||||
async fn write_workspace_fork(
|
||||
db: DB,
|
||||
@@ -10229,6 +10377,14 @@ async fn write_workspace_fork(
|
||||
}
|
||||
}
|
||||
|
||||
isolate_fork_storages(
|
||||
&mut tx,
|
||||
&forked_id,
|
||||
&nw.shared_storages,
|
||||
&nw.shared_ducklakes,
|
||||
)
|
||||
.await?;
|
||||
|
||||
// Lock the parent ("prod") so edits are funneled through this dev workspace.
|
||||
let locked_prod = nw.is_dev_workspace && (nw.lock_prod_deploy || nw.lock_prod_forking);
|
||||
if locked_prod {
|
||||
@@ -10276,6 +10432,9 @@ async fn write_workspace_fork(
|
||||
.await?;
|
||||
}
|
||||
tx.commit().await?;
|
||||
// Same for a mapping cached before the isolation rows were visible: identity, writing the
|
||||
// root's keys.
|
||||
windmill_common::fork_storage::invalidate_fork_storage_keys_cache(&forked_id);
|
||||
|
||||
// A pre-creation lookup could have cached an EMPTY ancestor chain for this id, which
|
||||
// would bypass ducklake fork isolation for the TTL. The same lookup could have cached the id
|
||||
|
||||
Reference in New Issue
Block a user