mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: walk the whole fork ancestry for app installations and fork conflicts (#11151)
* fix: walk the whole fork ancestry for app installations and fork conflicts Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs: describe the fork-conflict gate as ancestor-wide Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * chore: update ee-repo-ref to d252afcc80e77fcc4f9a2a346b80908c8605a6c0 This commit updates the EE repository reference after PR #803 was merged in windmill-ee-private. Previous ee-repo-ref: 5f68c8c351ffc92feccffe69a857b60be376464e New ee-repo-ref: d252afcc80e77fcc4f9a2a346b80908c8605a6c0 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Fable 5.1 <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 Fable 5.1
windmill-internal-app[bot]
parent
57a99f66a8
commit
73dc892f9c
@@ -1 +1 @@
|
||||
93433d7c9dc34f2c0f56a5297d3453aabd2f9472
|
||||
d252afcc80e77fcc4f9a2a346b80908c8605a6c0
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
-- A three-deep fork chain whose schedule rows were cloned down at fork time.
|
||||
-- The middle fork has since deleted its copy, so the leaf's schedule shares
|
||||
-- its cron only with the root — the shape a direct-parent check misses.
|
||||
|
||||
INSERT INTO workspace (id, name, owner, parent_workspace_id) VALUES
|
||||
('sfc-root', 'sfc-root', 'sfc-admin', NULL),
|
||||
('sfc-mid', 'sfc-mid', 'sfc-admin', 'sfc-root'),
|
||||
('sfc-leaf', 'sfc-leaf', 'sfc-admin', 'sfc-mid');
|
||||
|
||||
INSERT INTO workspace_key (workspace_id, kind, key) VALUES
|
||||
('sfc-root', 'cloud', 'sfc-root-key'),
|
||||
('sfc-mid', 'cloud', 'sfc-mid-key'),
|
||||
('sfc-leaf', 'cloud', 'sfc-leaf-key');
|
||||
|
||||
INSERT INTO workspace_settings (workspace_id) VALUES
|
||||
('sfc-root'), ('sfc-mid'), ('sfc-leaf');
|
||||
|
||||
INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
|
||||
('sfc-root', 'all', 'All users', '{}'),
|
||||
('sfc-mid', 'all', 'All users', '{}'),
|
||||
('sfc-leaf', 'all', 'All users', '{}');
|
||||
|
||||
INSERT INTO password(email, password_hash, login_type, super_admin, verified, name, username)
|
||||
VALUES ('sfc-admin@windmill.dev', 'x', 'password', true, true, 'SFC Admin', 'sfc-admin');
|
||||
|
||||
INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES
|
||||
('sfc-root', 'sfc-admin@windmill.dev', 'sfc-admin', true, 'Admin'),
|
||||
('sfc-mid', 'sfc-admin@windmill.dev', 'sfc-admin', true, 'Admin'),
|
||||
('sfc-leaf', 'sfc-admin@windmill.dev', 'sfc-admin', true, 'Admin');
|
||||
|
||||
INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin)
|
||||
VALUES (encode(sha256('SFC_ADMIN_TOKEN'::bytea), 'hex'), 'SFC_ADMIN_', 'SFC_ADMIN_TOKEN', 'sfc-admin@windmill.dev', 't', true);
|
||||
|
||||
-- Enabling pushes the next run, which needs the scheduled script to exist.
|
||||
INSERT INTO script (workspace_id, created_by, content, schema, summary, description, path, hash, language, lock) VALUES
|
||||
('sfc-leaf', 'sfc-admin', 'export async function main() { return "ok" }', '{}', '', '', 'f/shared/job', 7788001, 'deno', '');
|
||||
|
||||
INSERT INTO schedule (workspace_id, path, edited_by, edited_at, schedule, enabled, script_path, args, is_flow, email, timezone, extra_perms, permissioned_as)
|
||||
VALUES
|
||||
('sfc-root', 'f/shared/nightly', 'sfc-admin', NOW(), '0 0 0 * * *', true, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin'),
|
||||
('sfc-leaf', 'f/shared/nightly', 'sfc-admin', NOW(), '0 0 0 * * *', false, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin'),
|
||||
-- A path only the leaf has: nothing above shares it.
|
||||
('sfc-leaf', 'f/shared/own', 'sfc-admin', NOW(), '0 0 0 * * *', false, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin');
|
||||
|
||||
GRANT ALL PRIVILEGES ON TABLE workspace_key TO windmill_admin;
|
||||
GRANT ALL PRIVILEGES ON TABLE workspace_key TO windmill_user;
|
||||
@@ -12,8 +12,8 @@
|
||||
|
||||
use sqlx::{Pool, Postgres};
|
||||
use windmill_common::git_sync_ee::{
|
||||
create_repo_webhook, git_credential_for_url, repo_provider, repo_supports_managed_git_features,
|
||||
set_git_credential, GitProvider,
|
||||
create_repo_webhook, git_app_installations_for, git_credential_for_url, managed_pr_base_branch,
|
||||
repo_provider, repo_supports_managed_git_features, set_git_credential, GitProvider,
|
||||
};
|
||||
use windmill_common::workspaces::GitCredentialProvider;
|
||||
|
||||
@@ -278,3 +278,99 @@ async fn an_unreachable_gitlab_host_is_the_reported_error(
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// GitHub App installations are normally copied into a fork, but a workspace
|
||||
/// attached as a dev workspace, or forked before its parent connected the App,
|
||||
/// holds none, and neither does anything forked from it. The lookup reaches the
|
||||
/// nearest workspace up the chain that holds some, and the background App path
|
||||
/// (PR base resolution here) authenticates with that installation's token.
|
||||
#[sqlx::test(fixtures("git_sync_fork_credential"))]
|
||||
async fn app_installations_come_from_the_nearest_ancestor_holding_some(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
use axum::{routing::get, Router};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
// A stand-in GitHub API: one repository, and a record of who asked for it.
|
||||
let seen: Arc<Mutex<Vec<String>>> = Arc::new(Mutex::new(vec![]));
|
||||
let app = Router::new().route(
|
||||
"/api/v3/repos/acme/repo",
|
||||
get({
|
||||
let seen = seen.clone();
|
||||
move |headers: axum::http::HeaderMap| {
|
||||
let seen = seen.clone();
|
||||
async move {
|
||||
let auth = headers
|
||||
.get("authorization")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.unwrap_or("")
|
||||
.to_string();
|
||||
seen.lock().unwrap().push(auth);
|
||||
axum::Json(serde_json::json!({ "default_branch": "trunk" }))
|
||||
}
|
||||
}
|
||||
}),
|
||||
);
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?;
|
||||
let port = listener.local_addr()?.port();
|
||||
tokio::spawn(async move { axum::serve(listener, app).await.unwrap() });
|
||||
let stub = format!("http://127.0.0.1:{port}");
|
||||
|
||||
// The root holds the installation, with a cached token so nothing is minted.
|
||||
sqlx::query(
|
||||
"UPDATE workspace_settings SET git_app_installations = $1::jsonb WHERE workspace_id = 'parent-ws'",
|
||||
)
|
||||
.bind(serde_json::json!([{
|
||||
"installation_id": 42, "account_id": "acme", "jwt_token": "x",
|
||||
"github_base_url": stub,
|
||||
"installation_token": "root-token", "installation_token_expiration": 4102444800i64
|
||||
}]))
|
||||
.execute(&db)
|
||||
.await?;
|
||||
// The fork's copy of the resource names the App-backed repository.
|
||||
sqlx::query("UPDATE resource SET value = $1::jsonb WHERE workspace_id = 'deep-fork-ws' AND path = 'u/admin/repo'")
|
||||
.bind(serde_json::json!({ "url": format!("{stub}/acme/repo.git"), "is_github_app": true }))
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
git_app_installations_for(&db, "deep-fork-ws").await?,
|
||||
("parent-ws".to_string(), vec![(42, Some(stub.clone()))]),
|
||||
"two levels down, the root's installations are the ones to use"
|
||||
);
|
||||
assert_eq!(
|
||||
git_app_installations_for(&db, "orphan-ws").await?,
|
||||
("orphan-ws".to_string(), vec![]),
|
||||
"a workspace with nothing above it resolves nothing"
|
||||
);
|
||||
assert_eq!(
|
||||
managed_pr_base_branch(&db, "deep-fork-ws", REPO)
|
||||
.await?
|
||||
.as_deref(),
|
||||
Some("trunk"),
|
||||
"the background App path reaches the repository through the root's installation"
|
||||
);
|
||||
let seen = seen.lock().unwrap().clone();
|
||||
assert!(
|
||||
!seen.is_empty() && seen.iter().all(|auth| auth == "Bearer root-token"),
|
||||
"every call authenticated with the root's cached token: {seen:?}"
|
||||
);
|
||||
|
||||
// A closer holder takes precedence over the root.
|
||||
sqlx::query(
|
||||
"UPDATE workspace_settings SET git_app_installations = $1::jsonb WHERE workspace_id = 'fork-ws'",
|
||||
)
|
||||
.bind(serde_json::json!([{
|
||||
"installation_id": 7, "account_id": "acme", "jwt_token": "x",
|
||||
"github_base_url": stub,
|
||||
"installation_token": "mid-token", "installation_token_expiration": 4102444800i64
|
||||
}]))
|
||||
.execute(&db)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
git_app_installations_for(&db, "deep-fork-ws").await?.0,
|
||||
"fork-ws",
|
||||
"the nearest holder wins over the root"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
//! Enabling a schedule in a fork warns about every ancestor sharing the path,
|
||||
//! not only the direct parent: the row was cloned down the whole chain, so the
|
||||
//! cron is shared with whichever ancestors still hold a copy.
|
||||
|
||||
use serde_json::json;
|
||||
use sqlx::{Pool, Postgres};
|
||||
|
||||
use windmill_test_utils::*;
|
||||
|
||||
async fn set_enabled(
|
||||
base: &str,
|
||||
path: &str,
|
||||
enabled: bool,
|
||||
force: bool,
|
||||
) -> anyhow::Result<(u16, String)> {
|
||||
let resp = reqwest::Client::new()
|
||||
.post(format!("{base}/api/w/sfc-leaf/schedules/setenabled/{path}"))
|
||||
.header("Authorization", "Bearer SFC_ADMIN_TOKEN")
|
||||
.json(&json!({ "enabled": enabled, "force": force }))
|
||||
.send()
|
||||
.await?;
|
||||
Ok((resp.status().as_u16(), resp.text().await?))
|
||||
}
|
||||
|
||||
#[sqlx::test(fixtures("schedule_fork_conflict"))]
|
||||
async fn enabling_in_a_fork_names_the_nearest_ancestor_sharing_the_path(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let base = format!("http://localhost:{}", server.addr.port());
|
||||
|
||||
let (status, body) = set_enabled(&base, "f/shared/nightly", true, false).await?;
|
||||
assert_eq!(status, 400, "{body}");
|
||||
assert!(
|
||||
body.contains("fork-conflict:schedule:sfc-root"),
|
||||
"the middle fork deleted its copy, so the root is the one still sharing the cron: {body}"
|
||||
);
|
||||
|
||||
let (status, body) = set_enabled(&base, "f/shared/own", true, false).await?;
|
||||
assert_eq!(
|
||||
status, 200,
|
||||
"a path nothing upstream has enables freely: {body}"
|
||||
);
|
||||
|
||||
sqlx::query(
|
||||
"INSERT INTO schedule (workspace_id, path, edited_by, edited_at, schedule, enabled, script_path, args, is_flow, email, timezone, extra_perms, permissioned_as)
|
||||
VALUES ('sfc-mid', 'f/shared/nightly', 'sfc-admin', NOW(), '0 0 0 * * *', false, 'f/shared/job', '{}', false, 'sfc-admin@windmill.dev', 'UTC', '{}', 'u/sfc-admin')",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
let (status, body) = set_enabled(&base, "f/shared/nightly", true, false).await?;
|
||||
assert_eq!(status, 400, "{body}");
|
||||
assert!(
|
||||
body.contains("fork-conflict:schedule:sfc-mid"),
|
||||
"with the parent holding a copy again, it is the nearest and gets named: {body}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -1129,35 +1129,23 @@ pub async fn set_enabled(
|
||||
check_scopes(&authed, || format!("schedules:write:{}", path))?;
|
||||
reject_reserved_schedule_path(path)?;
|
||||
|
||||
// Block enabling a schedule in a fork when the parent has the same path
|
||||
// (regardless of parent's enabled flag), unless force=true. Two enabled
|
||||
// crons fire in lockstep; even when the parent is currently disabled the
|
||||
// user is likely to re-enable it later, at which point both fire — better
|
||||
// to surface that risk at every fork-side enable. There's no namespacing
|
||||
// fix for schedules (Phase 3 doesn't help cron); the user has to confirm
|
||||
// or point the script at fork-only side effects.
|
||||
// Block enabling a schedule in a fork when an ancestor has the same path
|
||||
// (regardless of its enabled flag), unless force=true. Two enabled crons
|
||||
// fire in lockstep; even when the ancestor is currently disabled the user
|
||||
// is likely to re-enable it later, at which point both fire — better to
|
||||
// surface that risk at every fork-side enable. There's no namespacing fix
|
||||
// for schedules (Phase 3 doesn't help cron); the user has to confirm or
|
||||
// point the script at fork-only side effects.
|
||||
if payload.enabled && !payload.force {
|
||||
let parent_id: Option<String> = sqlx::query_scalar!(
|
||||
"SELECT parent_workspace_id FROM workspace WHERE id = $1",
|
||||
&w_id
|
||||
if let Some(ancestor_id) = windmill_common::workspaces::nearest_fork_ancestor_having(
|
||||
&mut *tx, "schedule", &w_id, path,
|
||||
)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?
|
||||
.flatten();
|
||||
if let Some(parent_id) = parent_id {
|
||||
let exists: Option<bool> = sqlx::query_scalar!(
|
||||
"SELECT EXISTS(SELECT 1 FROM schedule WHERE workspace_id = $1 AND path = $2)",
|
||||
&parent_id,
|
||||
path,
|
||||
)
|
||||
.fetch_one(&mut *tx)
|
||||
.await?;
|
||||
if exists == Some(true) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"fork-conflict:schedule:{}",
|
||||
parent_id
|
||||
)));
|
||||
}
|
||||
{
|
||||
return Err(Error::BadRequest(format!(
|
||||
"fork-conflict:schedule:{}",
|
||||
ancestor_id
|
||||
)));
|
||||
}
|
||||
}
|
||||
let before = trigger_history::snapshot_row(&mut *tx, "schedule", &w_id, path).await?;
|
||||
@@ -1699,9 +1687,9 @@ pub use windmill_queue::schedule::clear_schedule;
|
||||
#[derive(Deserialize)]
|
||||
pub struct SetEnabled {
|
||||
pub enabled: bool,
|
||||
/// Bypass the parent-state warning when enabling a schedule in a fork
|
||||
/// whose parent has the same path enabled. The frontend sets this after
|
||||
/// the user confirms the duplicate-firing dialog.
|
||||
/// Bypass the fork-conflict warning when enabling a schedule in a fork
|
||||
/// while an ancestor workspace has the same path. The frontend sets this
|
||||
/// after the user confirms the duplicate-firing dialog.
|
||||
#[serde(default)]
|
||||
pub force: bool,
|
||||
}
|
||||
|
||||
@@ -1788,6 +1788,48 @@ pub async fn workspace_with_fork_ancestors(db: &crate::DB, w_id: &str) -> Result
|
||||
Ok(chain)
|
||||
}
|
||||
|
||||
/// The nearest fork ancestor of `w_id` holding a row at `path` in `table`, or `None` when no
|
||||
/// ancestor does (or `w_id` is not a fork). Fork creation clones trigger and schedule rows down
|
||||
/// the whole chain, so a row shares its upstream identifier (Kafka group, PG slot, cron) with
|
||||
/// every ancestor that still has one, not just the direct parent, which may have deleted its
|
||||
/// copy since.
|
||||
///
|
||||
/// Runs on the caller's connection so it sees the caller's transaction, and uncached: the
|
||||
/// answer depends on the target table, not only on lineage.
|
||||
///
|
||||
/// `table` is interpolated into SQL, hence `'static`: a trigger's `TABLE_NAME` or a literal,
|
||||
/// never caller input. Reads lineage for any `w_id` with no authorization check, like
|
||||
/// [`fork_ancestor_chain`], so the caller must already be authorized for `w_id`.
|
||||
pub async fn nearest_fork_ancestor_having(
|
||||
conn: &mut sqlx::PgConnection,
|
||||
table: &'static str,
|
||||
w_id: &str,
|
||||
path: &str,
|
||||
) -> Result<Option<String>> {
|
||||
sqlx::query_scalar(&format!(
|
||||
r#"
|
||||
WITH RECURSIVE chain AS (
|
||||
SELECT id, parent_workspace_id, 0 AS depth
|
||||
FROM workspace WHERE id = $1
|
||||
UNION ALL
|
||||
SELECT w.id, w.parent_workspace_id, chain.depth + 1
|
||||
FROM workspace w
|
||||
JOIN chain ON w.id = chain.parent_workspace_id
|
||||
WHERE chain.depth < 20
|
||||
)
|
||||
SELECT chain.id FROM chain
|
||||
JOIN {table} t ON t.workspace_id = chain.id AND t.path = $2
|
||||
WHERE chain.depth > 0
|
||||
ORDER BY chain.depth LIMIT 1
|
||||
"#
|
||||
))
|
||||
.bind(w_id)
|
||||
.bind(path)
|
||||
.fetch_optional(&mut *conn)
|
||||
.await
|
||||
.map_err(|e| Error::internal_err(format!("resolving fork ancestors of {w_id}: {e:#}")))
|
||||
}
|
||||
|
||||
lazy_static::lazy_static! {
|
||||
/// workspace id -> (root workspace id, expiry ts). Read once per job start, so correctness
|
||||
/// rests on the invalidation rather than on the TTL: every mutation that can change the answer
|
||||
|
||||
@@ -97,14 +97,14 @@ pub trait TriggerCrud: Send + Sync + 'static {
|
||||
const DEPLOYMENT_NAME: &'static str;
|
||||
const ADDITIONAL_SELECT_FIELDS: &[&'static str] = &[];
|
||||
const IS_ALLOWED_ON_CLOUD: bool;
|
||||
/// Whether enabling this trigger in a fork while the parent has the same
|
||||
/// path enabled is a real conflict (shared upstream resource). True for
|
||||
/// Whether enabling this trigger in a fork while an ancestor workspace has
|
||||
/// the same path is a real conflict (shared upstream resource). True for
|
||||
/// listener-based kinds where two consumers compete (Kafka group, PG slot,
|
||||
/// SQS queue, etc.) and for Websocket where both subscribers fire on every
|
||||
/// broadcast. False for kinds whose upstream identifier is implicitly
|
||||
/// workspace-scoped at runtime (HTTP routes, Email local_part — clones for
|
||||
/// the non-workspaced sub-case are filtered out, so any cloned row is
|
||||
/// already collision-free vs. the parent).
|
||||
/// already collision-free vs. its ancestors).
|
||||
const FORK_CONFLICT_ON_ENABLE: bool = true;
|
||||
|
||||
fn get_deployed_object(path: String, parent_path: Option<String>) -> DeployedObject;
|
||||
@@ -1095,54 +1095,14 @@ async fn exists_trigger<T: TriggerCrud>(
|
||||
#[derive(serde::Deserialize)]
|
||||
struct SetTriggerModePayload {
|
||||
mode: TriggerMode,
|
||||
/// When true, bypass the parent-state warning that would otherwise reject
|
||||
/// enabling a trigger that's already enabled in the parent workspace.
|
||||
/// The frontend sets this after the user confirms the duplicate-execution
|
||||
/// dialog. See windmill-trigger/src/handler.rs::set_trigger_mode for the
|
||||
/// full check.
|
||||
/// When true, bypass the fork-conflict warning that would otherwise reject
|
||||
/// enabling a trigger an ancestor workspace also has at this path. The
|
||||
/// frontend sets this after the user confirms the duplicate-execution
|
||||
/// dialog. See `set_trigger_mode` for the full check.
|
||||
#[serde(default)]
|
||||
force: bool,
|
||||
}
|
||||
|
||||
/// Returns the parent workspace id when this workspace is a fork *and* the
|
||||
/// parent has a row at the same trigger path. Used to gate enabling a trigger
|
||||
/// in a fork behind an explicit `force=true` confirmation: the fork's row was
|
||||
/// cloned from the parent, so its upstream identifier (Kafka group, PG slot,
|
||||
/// SQS queue URL, etc.) is shared by construction. The risk is independent of
|
||||
/// the parent's current `mode`: if the parent is enabled, the two listeners
|
||||
/// compete; if it's disabled, the fork can destructively take over shared
|
||||
/// state (e.g. advance the PG WAL, claim an MQTT client_id) before the parent
|
||||
/// re-enables. Either way, the user should be asked to confirm.
|
||||
async fn parent_has_trigger(
|
||||
tx: &mut PgConnection,
|
||||
table_name: &str,
|
||||
workspace_id: &str,
|
||||
path: &str,
|
||||
) -> Result<Option<String>> {
|
||||
let parent: Option<String> =
|
||||
sqlx::query_scalar("SELECT parent_workspace_id FROM workspace WHERE id = $1")
|
||||
.bind(workspace_id)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?
|
||||
.flatten();
|
||||
let Some(parent_id) = parent else {
|
||||
return Ok(None);
|
||||
};
|
||||
let exists: Option<bool> = sqlx::query_scalar(&format!(
|
||||
"SELECT EXISTS(SELECT 1 FROM {} WHERE workspace_id = $1 AND path = $2)",
|
||||
table_name
|
||||
))
|
||||
.bind(&parent_id)
|
||||
.bind(path)
|
||||
.fetch_one(&mut *tx)
|
||||
.await?;
|
||||
Ok(if exists == Some(true) {
|
||||
Some(parent_id)
|
||||
} else {
|
||||
None
|
||||
})
|
||||
}
|
||||
|
||||
async fn set_trigger_mode<T: TriggerCrud>(
|
||||
Extension(handler): Extension<Arc<T>>,
|
||||
authed: ApiAuthed,
|
||||
@@ -1157,22 +1117,28 @@ async fn set_trigger_mode<T: TriggerCrud>(
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
// Block transitioning a trigger in a fork to any mode that attaches a
|
||||
// listener (Enabled or Suspended) when the parent has the same path,
|
||||
// listener (Enabled or Suspended) when an ancestor has the same path,
|
||||
// unless the caller passes force=true. Suspended still keeps the
|
||||
// listener attached — it just stops auto-running queued jobs — so a
|
||||
// suspended fork would still split Kafka events / share a PG slot
|
||||
// with the parent. The cloned upstream identifier is shared by
|
||||
// construction; the risk is independent of the parent's current mode.
|
||||
// Skipped for kinds where the upstream identifier is already
|
||||
// workspace-scoped at runtime (HTTP, Email).
|
||||
// with the ancestor. The risk is independent of the ancestor's current
|
||||
// mode: enabled, the two listeners compete; disabled, the fork can
|
||||
// destructively take over shared state (advance the PG WAL, claim an
|
||||
// MQTT client_id) before it re-enables. Skipped for kinds where the
|
||||
// upstream identifier is already workspace-scoped at runtime (HTTP, Email).
|
||||
if T::FORK_CONFLICT_ON_ENABLE && payload.mode != TriggerMode::Disabled && !payload.force {
|
||||
if let Some(parent_id) =
|
||||
parent_has_trigger(&mut *tx, T::TABLE_NAME, &workspace_id, path).await?
|
||||
if let Some(ancestor_id) = windmill_common::workspaces::nearest_fork_ancestor_having(
|
||||
&mut *tx,
|
||||
T::TABLE_NAME,
|
||||
&workspace_id,
|
||||
path,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
return Err(Error::BadRequest(format!(
|
||||
"fork-conflict:{}:{}",
|
||||
T::TRIGGER_TYPE,
|
||||
parent_id
|
||||
ancestor_id
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,29 +32,29 @@
|
||||
|
||||
<ConfirmationModal
|
||||
open={!!state}
|
||||
title="Enable in fork conflicts with parent"
|
||||
title="Enable in fork conflicts with upstream workspace"
|
||||
confirmationText="Enable anyway"
|
||||
onConfirmed={() => close(true)}
|
||||
onCanceled={() => close(false)}
|
||||
>
|
||||
{#if state}
|
||||
<p>
|
||||
The parent workspace (<span class="font-mono">{state.parentWorkspaceId}</span>) has the same
|
||||
{state.kindLabel} configured at this path. Because this fork's row was cloned from it, the upstream
|
||||
identifier is shared.
|
||||
The upstream workspace (<span class="font-mono">{state.upstreamWorkspaceId}</span>) this fork
|
||||
descends from has the same {state.kindLabel} configured at this path. Because this fork's row was
|
||||
cloned from it, the upstream identifier is shared.
|
||||
</p>
|
||||
<p class="mt-2">
|
||||
{#if family === 'split'}
|
||||
If both are enabled, the two listeners will compete on the same upstream and each side will
|
||||
receive only a fraction of its events.
|
||||
{:else if family === 'duplicate'}
|
||||
If both are enabled, every event will fire the script twice — once in the fork and once in
|
||||
the parent.
|
||||
If both are enabled, every event will fire the script twice: once in the fork and once in
|
||||
the upstream workspace.
|
||||
{:else if family === 'slot'}
|
||||
The cloned <span class="font-mono">replication_slot_name</span> points at the same Postgres slot,
|
||||
which only allows one consumer at a time. Enabling here will either fail with "slot already active"
|
||||
if the parent is enabled, or hijack the slot's WAL position if it isn't — causing the parent
|
||||
to lose events when re-enabled.
|
||||
if the upstream workspace is enabled, or hijack the slot's WAL position if it isn't, causing
|
||||
it to lose events when re-enabled.
|
||||
{:else}
|
||||
Enabling it here may compete for the same upstream events or duplicate side effects.
|
||||
{/if}
|
||||
|
||||
@@ -242,7 +242,7 @@ export type GlobalForkModalState = {
|
||||
export type ForkConflictModalState = {
|
||||
kind: string
|
||||
kindLabel: string
|
||||
parentWorkspaceId: string
|
||||
upstreamWorkspaceId: string
|
||||
resolve: (proceed: boolean) => void
|
||||
}
|
||||
|
||||
|
||||
@@ -2,14 +2,15 @@ import { forkConflictModal } from '$lib/stores'
|
||||
|
||||
/**
|
||||
* The backend rejects "enable" requests on triggers/schedules in a fork when
|
||||
* the parent workspace has the same path enabled. The error body is shaped as
|
||||
* `fork-conflict:<kind>:<parent_workspace_id>`
|
||||
* an upstream workspace (the parent, or an ancestor further up) has the same
|
||||
* path. The error body is shaped as
|
||||
* `fork-conflict:<kind>:<upstream_workspace_id>`
|
||||
* so the UI can show a tailored confirm-to-proceed dialog and re-issue the
|
||||
* call with `force: true` if the user agrees.
|
||||
*/
|
||||
export interface ForkConflict {
|
||||
kind: string
|
||||
parentWorkspaceId: string
|
||||
upstreamWorkspaceId: string
|
||||
}
|
||||
|
||||
export function detectForkConflict(e: unknown): ForkConflict | null {
|
||||
@@ -20,7 +21,7 @@ export function detectForkConflict(e: unknown): ForkConflict | null {
|
||||
: ((body as any)?.error?.message ?? (body as any)?.message ?? (e as any)?.message ?? '')
|
||||
const m = String(raw).match(/fork-conflict:([^:]+):(.+)/)
|
||||
if (!m) return null
|
||||
return { kind: m[1], parentWorkspaceId: m[2].trim() }
|
||||
return { kind: m[1], upstreamWorkspaceId: m[2].trim() }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -30,11 +31,11 @@ export function detectForkConflict(e: unknown): ForkConflict | null {
|
||||
* on two rows in quick succession), resolve the older promise to false so
|
||||
* the prior caller doesn't hang.
|
||||
*/
|
||||
function askForkConflictConfirm(kind: string, kindLabel: string, parentWorkspaceId: string) {
|
||||
function askForkConflictConfirm(kind: string, kindLabel: string, upstreamWorkspaceId: string) {
|
||||
return new Promise<boolean>((resolve) => {
|
||||
const previous = forkConflictModal.val
|
||||
previous?.resolve(false)
|
||||
forkConflictModal.val = { kind, kindLabel, parentWorkspaceId, resolve }
|
||||
forkConflictModal.val = { kind, kindLabel, upstreamWorkspaceId, resolve }
|
||||
})
|
||||
}
|
||||
|
||||
@@ -64,7 +65,7 @@ export async function withForkConflictRetry(
|
||||
const proceed = await askForkConflictConfirm(
|
||||
conflict.kind,
|
||||
kindLabel,
|
||||
conflict.parentWorkspaceId
|
||||
conflict.upstreamWorkspaceId
|
||||
)
|
||||
// User explicitly dismissed the modal — treat as a silent no-op so the
|
||||
// caller's catch block doesn't pop a redundant error toast.
|
||||
|
||||
Reference in New Issue
Block a user