diff --git a/backend/.sqlx/query-2effe885aafaa0c5fc1f969b8c6ffbd38c53fb586251ba780b75a007e5a47ed6.json b/backend/.sqlx/query-4d102f01f0d41894fa2cd50475af59c6b4d6f170bce5daa12ef94ba1dcef06a3.json similarity index 84% rename from backend/.sqlx/query-2effe885aafaa0c5fc1f969b8c6ffbd38c53fb586251ba780b75a007e5a47ed6.json rename to backend/.sqlx/query-4d102f01f0d41894fa2cd50475af59c6b4d6f170bce5daa12ef94ba1dcef06a3.json index 6d948ee8ae..940b15e4bd 100644 --- a/backend/.sqlx/query-2effe885aafaa0c5fc1f969b8c6ffbd38c53fb586251ba780b75a007e5a47ed6.json +++ b/backend/.sqlx/query-4d102f01f0d41894fa2cd50475af59c6b4d6f170bce5daa12ef94ba1dcef06a3.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n INSERT INTO schedule (\n workspace_id, path, schedule, timezone, edited_by, script_path,\n is_flow, args, enabled, email, permissioned_as,\n on_failure, on_failure_times, on_failure_exact, on_failure_extra_args,\n on_recovery, on_recovery_times, on_recovery_extra_args,\n on_success, on_success_extra_args,\n ws_error_handler_muted, retry, summary, no_flow_overlap,\n tag, paused_until, cron_version, description, dynamic_skip, labels\n ) VALUES (\n $1, $2, $3, $4, $5, $6,\n $7, $8, $9, $10, $11,\n $12, $13, $14, $15,\n $16, $17, $18,\n $19, $20,\n $21, $22, $23, $24,\n $25, $26, $27, $28, $29, $30\n )\n RETURNING\n workspace_id,\n path,\n edited_by,\n edited_at,\n schedule,\n timezone,\n enabled,\n script_path,\n is_flow,\n args AS \"args: _\",\n extra_perms,\n email,\n permissioned_as,\n error,\n on_failure,\n on_failure_times,\n on_failure_exact,\n on_failure_extra_args AS \"on_failure_extra_args: _\",\n on_recovery,\n on_recovery_times,\n on_recovery_extra_args AS \"on_recovery_extra_args: _\",\n on_success,\n on_success_extra_args AS \"on_success_extra_args: _\",\n ws_error_handler_muted,\n retry,\n no_flow_overlap,\n summary,\n description,\n tag,\n paused_until,\n cron_version,\n dynamic_skip,\n labels\n ", + "query": "\n INSERT INTO schedule (\n workspace_id, path, schedule, timezone, edited_by, script_path,\n is_flow, args, enabled, permissioned_as, email,\n on_failure, on_failure_times, on_failure_exact, on_failure_extra_args,\n on_recovery, on_recovery_times, on_recovery_extra_args,\n on_success, on_success_extra_args,\n ws_error_handler_muted, retry, summary, no_flow_overlap,\n tag, paused_until, cron_version, description, dynamic_skip, labels\n ) VALUES (\n $1, $2, $3, $4, $5, $6,\n $7, $8, $9, $10, $11,\n $12, $13, $14, $15,\n $16, $17, $18,\n $19, $20,\n $21, $22, $23, $24,\n $25, $26, $27, $28, $29, $30\n )\n RETURNING\n workspace_id,\n path,\n edited_by,\n edited_at,\n schedule,\n timezone,\n enabled,\n script_path,\n is_flow,\n args AS \"args: _\",\n extra_perms,\n permissioned_as,\n error,\n on_failure,\n on_failure_times,\n on_failure_exact,\n on_failure_extra_args AS \"on_failure_extra_args: _\",\n on_recovery,\n on_recovery_times,\n on_recovery_extra_args AS \"on_recovery_extra_args: _\",\n on_success,\n on_success_extra_args AS \"on_success_extra_args: _\",\n ws_error_handler_muted,\n retry,\n no_flow_overlap,\n summary,\n description,\n tag,\n paused_until,\n cron_version,\n dynamic_skip,\n labels\n ", "describe": { "columns": [ { @@ -60,111 +60,106 @@ }, { "ordinal": 11, - "name": "email", - "type_info": "Varchar" - }, - { - "ordinal": 12, "name": "permissioned_as", "type_info": "Varchar" }, { - "ordinal": 13, + "ordinal": 12, "name": "error", "type_info": "Text" }, { - "ordinal": 14, + "ordinal": 13, "name": "on_failure", "type_info": "Varchar" }, { - "ordinal": 15, + "ordinal": 14, "name": "on_failure_times", "type_info": "Int4" }, { - "ordinal": 16, + "ordinal": 15, "name": "on_failure_exact", "type_info": "Bool" }, { - "ordinal": 17, + "ordinal": 16, "name": "on_failure_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 18, + "ordinal": 17, "name": "on_recovery", "type_info": "Varchar" }, { - "ordinal": 19, + "ordinal": 18, "name": "on_recovery_times", "type_info": "Int4" }, { - "ordinal": 20, + "ordinal": 19, "name": "on_recovery_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 21, + "ordinal": 20, "name": "on_success", "type_info": "Varchar" }, { - "ordinal": 22, + "ordinal": 21, "name": "on_success_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 23, + "ordinal": 22, "name": "ws_error_handler_muted", "type_info": "Bool" }, { - "ordinal": 24, + "ordinal": 23, "name": "retry", "type_info": "Jsonb" }, { - "ordinal": 25, + "ordinal": 24, "name": "no_flow_overlap", "type_info": "Bool" }, { - "ordinal": 26, + "ordinal": 25, "name": "summary", "type_info": "Varchar" }, { - "ordinal": 27, + "ordinal": 26, "name": "description", "type_info": "Text" }, { - "ordinal": 28, + "ordinal": 27, "name": "tag", "type_info": "Varchar" }, { - "ordinal": 29, + "ordinal": 28, "name": "paused_until", "type_info": "Timestamptz" }, { - "ordinal": 30, + "ordinal": 29, "name": "cron_version", "type_info": "Text" }, { - "ordinal": 31, + "ordinal": 30, "name": "dynamic_skip", "type_info": "Varchar" }, { - "ordinal": 32, + "ordinal": 31, "name": "labels", "type_info": "TextArray" } @@ -216,7 +211,6 @@ true, false, false, - false, true, true, true, @@ -239,5 +233,5 @@ true ] }, - "hash": "2effe885aafaa0c5fc1f969b8c6ffbd38c53fb586251ba780b75a007e5a47ed6" + "hash": "4d102f01f0d41894fa2cd50475af59c6b4d6f170bce5daa12ef94ba1dcef06a3" } diff --git a/backend/.sqlx/query-bd2609f999054c5a2a85f2aafe4005bc22731c27f0bf27b3b06ec426a05eda2e.json b/backend/.sqlx/query-6d4128dbebbfa3cf618b7666d6e9008e795153301fd29df74db36721be970eb5.json similarity index 83% rename from backend/.sqlx/query-bd2609f999054c5a2a85f2aafe4005bc22731c27f0bf27b3b06ec426a05eda2e.json rename to backend/.sqlx/query-6d4128dbebbfa3cf618b7666d6e9008e795153301fd29df74db36721be970eb5.json index 578edc6044..10a8fc3dbe 100644 --- a/backend/.sqlx/query-bd2609f999054c5a2a85f2aafe4005bc22731c27f0bf27b3b06ec426a05eda2e.json +++ b/backend/.sqlx/query-6d4128dbebbfa3cf618b7666d6e9008e795153301fd29df74db36721be970eb5.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n UPDATE schedule SET\n schedule = $1,\n timezone = $2,\n args = $3,\n on_failure = $4,\n on_failure_times = $5,\n on_failure_exact = $6,\n on_failure_extra_args = $7,\n on_recovery = $8,\n on_recovery_times = $9,\n on_recovery_extra_args = $10,\n on_success = $11,\n on_success_extra_args = $12,\n ws_error_handler_muted = $13,\n retry = $14,\n summary = $15,\n no_flow_overlap = $16,\n tag = $17,\n paused_until = $18,\n path = $19,\n workspace_id = $20,\n cron_version = COALESCE($21, cron_version),\n description = $22,\n dynamic_skip = $23,\n email = $24,\n edited_by = $25,\n permissioned_as = $26,\n labels = COALESCE($27, labels)\n WHERE path = $19 AND workspace_id = $20\n RETURNING\n workspace_id,\n path,\n edited_by,\n edited_at,\n schedule,\n timezone,\n enabled,\n script_path,\n is_flow,\n args AS \"args: _\",\n extra_perms,\n email,\n permissioned_as,\n error,\n on_failure,\n on_failure_times,\n on_failure_exact,\n on_failure_extra_args AS \"on_failure_extra_args: _\",\n on_recovery,\n on_recovery_times,\n on_recovery_extra_args AS \"on_recovery_extra_args: _\",\n on_success,\n on_success_extra_args AS \"on_success_extra_args: _\",\n ws_error_handler_muted,\n retry,\n no_flow_overlap,\n summary,\n description,\n tag,\n paused_until,\n cron_version,\n dynamic_skip,\n labels\n ", + "query": "\n UPDATE schedule SET\n schedule = $1,\n timezone = $2,\n args = $3,\n on_failure = $4,\n on_failure_times = $5,\n on_failure_exact = $6,\n on_failure_extra_args = $7,\n on_recovery = $8,\n on_recovery_times = $9,\n on_recovery_extra_args = $10,\n on_success = $11,\n on_success_extra_args = $12,\n ws_error_handler_muted = $13,\n retry = $14,\n summary = $15,\n no_flow_overlap = $16,\n tag = $17,\n paused_until = $18,\n path = $19,\n workspace_id = $20,\n cron_version = COALESCE($21, cron_version),\n description = $22,\n dynamic_skip = $23,\n edited_by = $24,\n permissioned_as = $25,\n email = $26,\n labels = COALESCE($27, labels)\n WHERE path = $19 AND workspace_id = $20\n RETURNING\n workspace_id,\n path,\n edited_by,\n edited_at,\n schedule,\n timezone,\n enabled,\n script_path,\n is_flow,\n args AS \"args: _\",\n extra_perms,\n permissioned_as,\n error,\n on_failure,\n on_failure_times,\n on_failure_exact,\n on_failure_extra_args AS \"on_failure_extra_args: _\",\n on_recovery,\n on_recovery_times,\n on_recovery_extra_args AS \"on_recovery_extra_args: _\",\n on_success,\n on_success_extra_args AS \"on_success_extra_args: _\",\n ws_error_handler_muted,\n retry,\n no_flow_overlap,\n summary,\n description,\n tag,\n paused_until,\n cron_version,\n dynamic_skip,\n labels\n ", "describe": { "columns": [ { @@ -60,111 +60,106 @@ }, { "ordinal": 11, - "name": "email", - "type_info": "Varchar" - }, - { - "ordinal": 12, "name": "permissioned_as", "type_info": "Varchar" }, { - "ordinal": 13, + "ordinal": 12, "name": "error", "type_info": "Text" }, { - "ordinal": 14, + "ordinal": 13, "name": "on_failure", "type_info": "Varchar" }, { - "ordinal": 15, + "ordinal": 14, "name": "on_failure_times", "type_info": "Int4" }, { - "ordinal": 16, + "ordinal": 15, "name": "on_failure_exact", "type_info": "Bool" }, { - "ordinal": 17, + "ordinal": 16, "name": "on_failure_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 18, + "ordinal": 17, "name": "on_recovery", "type_info": "Varchar" }, { - "ordinal": 19, + "ordinal": 18, "name": "on_recovery_times", "type_info": "Int4" }, { - "ordinal": 20, + "ordinal": 19, "name": "on_recovery_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 21, + "ordinal": 20, "name": "on_success", "type_info": "Varchar" }, { - "ordinal": 22, + "ordinal": 21, "name": "on_success_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 23, + "ordinal": 22, "name": "ws_error_handler_muted", "type_info": "Bool" }, { - "ordinal": 24, + "ordinal": 23, "name": "retry", "type_info": "Jsonb" }, { - "ordinal": 25, + "ordinal": 24, "name": "no_flow_overlap", "type_info": "Bool" }, { - "ordinal": 26, + "ordinal": 25, "name": "summary", "type_info": "Varchar" }, { - "ordinal": 27, + "ordinal": 26, "name": "description", "type_info": "Text" }, { - "ordinal": 28, + "ordinal": 27, "name": "tag", "type_info": "Varchar" }, { - "ordinal": 29, + "ordinal": 28, "name": "paused_until", "type_info": "Timestamptz" }, { - "ordinal": 30, + "ordinal": 29, "name": "cron_version", "type_info": "Text" }, { - "ordinal": 31, + "ordinal": 30, "name": "dynamic_skip", "type_info": "Varchar" }, { - "ordinal": 32, + "ordinal": 31, "name": "labels", "type_info": "TextArray" } @@ -213,7 +208,6 @@ true, false, false, - false, true, true, true, @@ -236,5 +230,5 @@ true ] }, - "hash": "bd2609f999054c5a2a85f2aafe4005bc22731c27f0bf27b3b06ec426a05eda2e" + "hash": "6d4128dbebbfa3cf618b7666d6e9008e795153301fd29df74db36721be970eb5" } diff --git a/backend/.sqlx/query-642ca096c1c151fc01445593c8d2ed879a9a7be6ff044a9ce1b581ba725665fe.json b/backend/.sqlx/query-830dca285c094f17f76a42e3621b9bf8d5ac930673dd5d5793f1102da85a9f6c.json similarity index 75% rename from backend/.sqlx/query-642ca096c1c151fc01445593c8d2ed879a9a7be6ff044a9ce1b581ba725665fe.json rename to backend/.sqlx/query-830dca285c094f17f76a42e3621b9bf8d5ac930673dd5d5793f1102da85a9f6c.json index a205bcbbe6..e17e52a1ec 100644 --- a/backend/.sqlx/query-642ca096c1c151fc01445593c8d2ed879a9a7be6ff044a9ce1b581ba725665fe.json +++ b/backend/.sqlx/query-830dca285c094f17f76a42e3621b9bf8d5ac930673dd5d5793f1102da85a9f6c.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n UPDATE schedule SET\n enabled = $1,\n email = $2\n WHERE path = $3 AND workspace_id = $4\n RETURNING\n workspace_id,\n path,\n edited_by,\n edited_at,\n schedule,\n timezone,\n enabled,\n script_path,\n is_flow,\n args AS \"args: _\",\n extra_perms,\n email,\n permissioned_as,\n error,\n on_failure,\n on_failure_times,\n on_failure_exact,\n on_failure_extra_args AS \"on_failure_extra_args: _\",\n on_recovery,\n on_recovery_times,\n on_recovery_extra_args AS \"on_recovery_extra_args: _\",\n on_success,\n on_success_extra_args AS \"on_success_extra_args: _\",\n ws_error_handler_muted,\n retry,\n no_flow_overlap,\n summary,\n description,\n tag,\n paused_until,\n cron_version,\n dynamic_skip,\n labels\n ", + "query": "\n UPDATE schedule SET\n enabled = $1\n WHERE path = $2 AND workspace_id = $3\n RETURNING\n workspace_id,\n path,\n edited_by,\n edited_at,\n schedule,\n timezone,\n enabled,\n script_path,\n is_flow,\n args AS \"args: _\",\n extra_perms,\n permissioned_as,\n error,\n on_failure,\n on_failure_times,\n on_failure_exact,\n on_failure_extra_args AS \"on_failure_extra_args: _\",\n on_recovery,\n on_recovery_times,\n on_recovery_extra_args AS \"on_recovery_extra_args: _\",\n on_success,\n on_success_extra_args AS \"on_success_extra_args: _\",\n ws_error_handler_muted,\n retry,\n no_flow_overlap,\n summary,\n description,\n tag,\n paused_until,\n cron_version,\n dynamic_skip,\n labels\n ", "describe": { "columns": [ { @@ -60,111 +60,106 @@ }, { "ordinal": 11, - "name": "email", - "type_info": "Varchar" - }, - { - "ordinal": 12, "name": "permissioned_as", "type_info": "Varchar" }, { - "ordinal": 13, + "ordinal": 12, "name": "error", "type_info": "Text" }, { - "ordinal": 14, + "ordinal": 13, "name": "on_failure", "type_info": "Varchar" }, { - "ordinal": 15, + "ordinal": 14, "name": "on_failure_times", "type_info": "Int4" }, { - "ordinal": 16, + "ordinal": 15, "name": "on_failure_exact", "type_info": "Bool" }, { - "ordinal": 17, + "ordinal": 16, "name": "on_failure_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 18, + "ordinal": 17, "name": "on_recovery", "type_info": "Varchar" }, { - "ordinal": 19, + "ordinal": 18, "name": "on_recovery_times", "type_info": "Int4" }, { - "ordinal": 20, + "ordinal": 19, "name": "on_recovery_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 21, + "ordinal": 20, "name": "on_success", "type_info": "Varchar" }, { - "ordinal": 22, + "ordinal": 21, "name": "on_success_extra_args: _", "type_info": "Jsonb" }, { - "ordinal": 23, + "ordinal": 22, "name": "ws_error_handler_muted", "type_info": "Bool" }, { - "ordinal": 24, + "ordinal": 23, "name": "retry", "type_info": "Jsonb" }, { - "ordinal": 25, + "ordinal": 24, "name": "no_flow_overlap", "type_info": "Bool" }, { - "ordinal": 26, + "ordinal": 25, "name": "summary", "type_info": "Varchar" }, { - "ordinal": 27, + "ordinal": 26, "name": "description", "type_info": "Text" }, { - "ordinal": 28, + "ordinal": 27, "name": "tag", "type_info": "Varchar" }, { - "ordinal": 29, + "ordinal": 28, "name": "paused_until", "type_info": "Timestamptz" }, { - "ordinal": 30, + "ordinal": 29, "name": "cron_version", "type_info": "Text" }, { - "ordinal": 31, + "ordinal": 30, "name": "dynamic_skip", "type_info": "Varchar" }, { - "ordinal": 32, + "ordinal": 31, "name": "labels", "type_info": "TextArray" } @@ -172,7 +167,6 @@ "parameters": { "Left": [ "Bool", - "Varchar", "Text", "Text" ] @@ -190,7 +184,6 @@ true, false, false, - false, true, true, true, @@ -213,5 +206,5 @@ true ] }, - "hash": "642ca096c1c151fc01445593c8d2ed879a9a7be6ff044a9ce1b581ba725665fe" + "hash": "830dca285c094f17f76a42e3621b9bf8d5ac930673dd5d5793f1102da85a9f6c" } diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index c69a2444aa..a97d6006c8 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -0373b4bfdaf8dd51533552e2e4de63ceb3c18b4d +40d95414cc3e637f3a940c15a89f413e15a5599f diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index d8d1485b4a..15eded753f 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -2442,6 +2442,20 @@ async fn test_schedule_group_permissioned_as(db: Pool) -> anyhow::Resu "edited_by should be the deploying user, not the group" ); + // A group has no address of its own, so the synthetic one can only come from deriving it. + let resp = authed( + client().get(format!("{base}/schedules/get/u/test-user/schedule_group_perm")), + "SECRET_TOKEN", + ) + .send() + .await?; + let returned: serde_json::Value = resp.json().await?; + assert_eq!( + returned["email"].as_str(), + Some("group-all@windmill.dev"), + "the schedule response derives the address from the principal" + ); + Ok(()) } diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index e988430160..7944d8ea82 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -37,7 +37,7 @@ use windmill_common::{ worker::to_raw_value, }; use windmill_git_sync::{handle_deployment_metadata, DeployedObject}; -use windmill_queue::schedule::push_scheduled_job; +use windmill_queue::schedule::{push_scheduled_job, with_derived_email}; /// Resolves the permissioned_as value for a schedule. /// When preserving, uses the provided permissioned_as value directly. @@ -300,8 +300,10 @@ async fn create_schedule( &w_id, ) .await?; - // email is still written for backwards compat with old workers that don't know about permissioned_as - let resolved_email = windmill_common::users::get_email_from_permissioned_as_uncached( + // Uncached: unlike a read, this value is persisted, so a stale cached address would stay + // wrong in the row instead of for the minute the cache lives — and `change_user_email` + // sweeps the column, which a cached write would immediately undo. + let legacy_email = windmill_common::users::get_email_from_permissioned_as_uncached( &resolved_permissioned_as, &w_id, &db, @@ -313,7 +315,7 @@ async fn create_schedule( r#" INSERT INTO schedule ( workspace_id, path, schedule, timezone, edited_by, script_path, - is_flow, args, enabled, email, permissioned_as, + is_flow, args, enabled, permissioned_as, email, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, @@ -340,7 +342,6 @@ async fn create_schedule( is_flow, args AS "args: _", extra_perms, - email, permissioned_as, error, on_failure, @@ -384,8 +385,8 @@ async fn create_schedule( } else { ns.enabled.unwrap_or(true) }, - resolved_email, resolved_permissioned_as, + legacy_email, ns.on_failure, ns.on_failure_times, ns.on_failure_exact, @@ -505,20 +506,15 @@ async fn edit_schedule( &authed, ); - // email is still written for backwards compat with old workers that don't know about permissioned_as. - // When permissioned_as is preserved to a different user, derive email from it. - let resolved_email = if resolved_permissioned_as - != windmill_common::users::username_to_permissioned_as(&authed.username) - { - windmill_common::users::get_email_from_permissioned_as_uncached( - &resolved_permissioned_as, - &w_id, - &db, - ) - .await? - } else { - authed.email.clone() - }; + // Uncached: unlike a read, this value is persisted, so a stale cached address would stay + // wrong in the row instead of for the minute the cache lives — and `change_user_email` + // sweeps the column, which a cached write would immediately undo. + let legacy_email = windmill_common::users::get_email_from_permissioned_as_uncached( + &resolved_permissioned_as, + &w_id, + &db, + ) + .await?; let schedule = sqlx::query_as!( Schedule, @@ -547,9 +543,9 @@ async fn edit_schedule( cron_version = COALESCE($21, cron_version), description = $22, dynamic_skip = $23, - email = $24, - edited_by = $25, - permissioned_as = $26, + edited_by = $24, + permissioned_as = $25, + email = $26, labels = COALESCE($27, labels) WHERE path = $19 AND workspace_id = $20 RETURNING @@ -564,7 +560,6 @@ async fn edit_schedule( is_flow, args AS "args: _", extra_perms, - email, permissioned_as, error, on_failure, @@ -614,9 +609,9 @@ async fn edit_schedule( es.cron_version, es.description, es.dynamic_skip, - resolved_email, resolved_edited_by, resolved_permissioned_as, + legacy_email, es.labels.as_deref() as Option<&[String]> ) .fetch_one(&mut *tx) @@ -992,6 +987,10 @@ async fn get_schedule( let schedule_o = windmill_queue::schedule::get_schedule_opt(&mut *tx, &w_id, path).await?; tx.commit().await?; + let schedule_o = match schedule_o { + Some(schedule) => Some(with_derived_email(&db, &w_id, schedule).await?), + None => None, + }; let overlay = overlay_or_draft_only( &db, &w_id, @@ -1080,14 +1079,15 @@ pub async fn set_enabled( } } } - // email is still written for backwards compat with old workers that don't know about permissioned_as + // Only `enabled` moves. The stored address belongs to the schedule's identity, which + // toggling does not change, so stamping the toggling user's here would leave it naming a + // different account than the principal beside it. let schedule_o = sqlx::query_as!( Schedule, r#" UPDATE schedule SET - enabled = $1, - email = $2 - WHERE path = $3 AND workspace_id = $4 + enabled = $1 + WHERE path = $2 AND workspace_id = $3 RETURNING workspace_id, path, @@ -1100,7 +1100,6 @@ pub async fn set_enabled( is_flow, args AS "args: _", extra_perms, - email, permissioned_as, error, on_failure, @@ -1124,7 +1123,6 @@ pub async fn set_enabled( labels "#, payload.enabled, - authed.email, path, w_id ) diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 3bd9668ede..6b1aa15b54 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -1978,7 +1978,8 @@ async fn change_user_email( .execute(&mut *tx) .await?; - // ---- runnables run on behalf of the user ---- + // Still read by workers on the previous version (the schedule error handler passes it + // through), so it stays correct across a rolling deploy. sqlx::query!( "UPDATE schedule SET email = $1 WHERE email = $2", &new_email, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 84c69a5ecf..c490bedbed 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -27894,7 +27894,7 @@ components: description: Additional permissions for this schedule email: type: string - description: Email of the user who owns this schedule, used for permissioned_as + description: Address of `permissioned_as`. Derived from it when this endpoint answers, and written through from it on save, so the two always name the same account. permissioned_as: type: string description: The user or group this schedule runs as (e.g., 'u/admin' or 'g/mygroup') diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index ea3dc89a28..709ef6badd 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -52,7 +52,7 @@ use windmill_common::{ db::UserDB, error::{to_anyhow, Error, Result}, flows::Flow, - schedule::Schedule, + schedule::ScheduleWithEmail, scripts::{Schema, Script, ScriptLang}, variables::{build_crypt, ExportableListableVariable}, workspace_dependencies::WorkspaceDependencies, @@ -737,8 +737,7 @@ pub(crate) async fn tarball_workspace( // From v1 the CLI never writes the address, so resolving it would be pure waste on the // default sync path; emit the marker it actually keeps instead. - let obo_marker_only = - parse_sync_behavior_version(sync_behavior_version.as_deref()) >= 1; + let obo_marker_only = parse_sync_behavior_version(sync_behavior_version.as_deref()) >= 1; let mut obo_cache: HashMap = HashMap::new(); { let scripts = sqlx::query_as::<_, Script>(&format!( @@ -1029,8 +1028,8 @@ pub(crate) async fn tarball_workspace( // derived from the workspace ducklake settings (and admins bypass the // RLS that hides them), so exporting them would drag unsyncable rows // into git. - let schedules = sqlx::query_as::<_, Schedule>( - "SELECT workspace_id, path, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, args, extra_perms, email, permissioned_as, error, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, ws_error_handler_muted, retry, no_flow_overlap, summary, description, tag, paused_until, cron_version, dynamic_skip, labels FROM schedule + let schedules = sqlx::query_as::<_, ScheduleWithEmail>( + "SELECT workspace_id, path, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, args, extra_perms, permissioned_as, email, error, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, ws_error_handler_muted, retry, no_flow_overlap, summary, description, tag, paused_until, cron_version, dynamic_skip, labels FROM schedule WHERE workspace_id = $1 AND NOT starts_with(path, $2)", ) .bind(&w_id) @@ -1042,21 +1041,26 @@ pub(crate) async fn tarball_workspace( // synced file matches the parent and the merge doesn't flip it. let parent_enabled = fork_parent_schedule_enabled(&db, parent_workspace_id.as_deref()).await?; - for schedule in schedules { - let enabled_override = parent_enabled.get(&schedule.path).map(|enabled| { + // The stored address, not a fresh derivation: the row still carries what this release + // wrote through from the principal, and re-deriving would turn a principal whose account + // has since been removed into a synthetic `@unknown.windmill.dev` where the exported file + // used to hold the real address. + for sched in schedules { + let enabled_override = parent_enabled.get(&sched.schedule.path).map(|enabled| { let mut o = serde_json::Map::new(); o.insert("enabled".to_string(), Value::Bool(*enabled)); o }); + let path = sched.schedule.path.clone(); let app_str = &to_string_without_metadata_inner( - &schedule, + &sched, ExtraPermsBehavior::Drop, None, enabled_override.as_ref(), ) .unwrap(); archive - .write_to_archive(&app_str, &format!("{}.schedule.json", schedule.path)) + .write_to_archive(&app_str, &format!("{}.schedule.json", path)) .await?; } } diff --git a/backend/windmill-common/src/min_version.rs b/backend/windmill-common/src/min_version.rs index 80b3871f3d..73de3d37c3 100644 --- a/backend/windmill-common/src/min_version.rs +++ b/backend/windmill-common/src/min_version.rs @@ -8,6 +8,36 @@ use semver::Version; // reads that column is live, and those runnables run as their deployer with no error anywhere. pub const MIN_VERSION_SUPPORTS_ON_BEHALF_OF_PRINCIPAL: VC = vc(1, 776, 0, "On-behalf-of principal"); + +/// Names the release that stops reading `schedule.email`. Errs a minor high for the same reason +/// as the constant above: the column is still written for the workers below this version whose +/// `get_schedule_opt` selects it, inside the same transaction as the job completion — a missing +/// column would roll that completion back and leave the occurrence to be re-executed. +/// +/// Nothing gates on this at runtime; it exists so `vc()`'s compile-time assert fires once +/// `MIN_KEEP_ALIVE_VERSION` passes it. When this constraint stops compiling, no supported worker +/// reads the column — but the removal still takes two releases, because the last readers are +/// this codebase's own and a rolling deploy runs both versions at once. +/// +/// Release A, code only, column untouched: +/// +/// 1. `workspaces_export.rs`'s schedule `SELECT`, which still names `email` and hydrates +/// `ScheduleWithEmail` — the only reader left, and the only one that would fail at runtime +/// rather than at compile time. It emits the derived address instead, like `get_schedule`. +/// 2. the writes in `windmill-api-schedule` (`create_schedule`, `edit_schedule`) and the clone +/// in `workspaces.rs`, plus the EE ducklake-maintenance upsert. +/// 3. the `UPDATE schedule SET email` sweep in `change_user_email`. +/// +/// Release B, once every replica runs A: +/// +/// 4. `ALTER TABLE schedule DROP COLUMN email`. `ScheduleWithEmail::email` stays — it is a +/// `required` field of the `Schedule` response schema, and by then every path fills it by +/// deriving, so only its source changes. +/// +/// Dropping the column in release A would break the replicas still on this one, which is the +/// same rolling-deploy hazard that made the column worth keeping in the first place. +pub const MIN_VERSION_DERIVES_SCHEDULE_EMAIL: VC = + vc(1, 777, 0, "Schedule email derived from permissioned_as"); pub const MIN_VERSION_SUPPORTS_NODE_DEBOUNCING: VC = vc(1, 658, 0, "Flow node debouncing"); pub const MIN_VERSION_SUPPORTS_TOKEN_HASH: VC = vc(1, 659, 0, "Token hash storage"); pub const MIN_VERSION_SUPPORTS_SYNC_JOBS_DEBOUNCING: VC = vc(1, 602, 0, "Sync jobs debouncing"); diff --git a/backend/windmill-queue/src/schedule.rs b/backend/windmill-queue/src/schedule.rs index 10f4a2b408..b966a8b690 100644 --- a/backend/windmill-queue/src/schedule.rs +++ b/backend/windmill-queue/src/schedule.rs @@ -34,7 +34,7 @@ use windmill_common::FlowVersionInfo; use windmill_common::DB; use windmill_common::{ error::{self, Result}, - schedule::Schedule, + schedule::{Schedule, ScheduleWithEmail}, utils::{now_from_db, ScheduleType, StripPath}, }; @@ -640,7 +640,7 @@ pub async fn rearm_schedule(db: &DB, w_id: &str, path: &str) -> Result( - "SELECT workspace_id, path, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, args, extra_perms, email, permissioned_as, error, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, ws_error_handler_muted, retry, no_flow_overlap, summary, description, tag, paused_until, cron_version, dynamic_skip, labels FROM schedule WHERE path = $1 AND workspace_id = $2 FOR UPDATE", + "SELECT workspace_id, path, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, args, extra_perms, permissioned_as, error, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, ws_error_handler_muted, retry, no_flow_overlap, summary, description, tag, paused_until, cron_version, dynamic_skip, labels FROM schedule WHERE path = $1 AND workspace_id = $2 FOR UPDATE", ) .bind(path) .bind(w_id) @@ -699,7 +699,7 @@ pub async fn get_schedule_opt<'c>( path: &str, ) -> Result> { let schedule_opt = sqlx::query_as::<_, Schedule>( - "SELECT workspace_id, path, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, args, extra_perms, email, permissioned_as, error, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, ws_error_handler_muted, retry, no_flow_overlap, summary, description, tag, paused_until, cron_version, dynamic_skip, labels FROM schedule WHERE path = $1 AND workspace_id = $2", + "SELECT workspace_id, path, edited_by, edited_at, schedule, timezone, enabled, script_path, is_flow, args, extra_perms, permissioned_as, error, on_failure, on_failure_times, on_failure_exact, on_failure_extra_args, on_recovery, on_recovery_times, on_recovery_extra_args, on_success, on_success_extra_args, ws_error_handler_muted, retry, no_flow_overlap, summary, description, tag, paused_until, cron_version, dynamic_skip, labels FROM schedule WHERE path = $1 AND workspace_id = $2", ) .bind(path) .bind(w_id) @@ -708,6 +708,24 @@ pub async fn get_schedule_opt<'c>( Ok(schedule_opt) } +/// Attach the address the schedule's `permissioned_as` resolves to, for the read paths whose +/// response shape has always carried one. The identity a schedule runs as is its principal +/// alone; the address is never read back from the row. +/// +/// Not an authorization boundary: it resolves an address for any `(w_id, schedule)` handed to +/// it, so a caller acting for a user MUST already have established their read access to that +/// schedule. +pub async fn with_derived_email<'c>( + e: impl sqlx::PgExecutor<'c>, + w_id: &str, + schedule: Schedule, +) -> Result { + let email = + windmill_common::users::get_email_from_permissioned_as(&schedule.permissioned_as, w_id, e) + .await?; + Ok(ScheduleWithEmail { schedule, email }) +} + pub async fn exists_schedule( tx: &mut Transaction<'_, Postgres>, w_id: String, diff --git a/backend/windmill-queue/tests/schedule_push.rs b/backend/windmill-queue/tests/schedule_push.rs index 2069ce06b7..361061b1a1 100644 --- a/backend/windmill-queue/tests/schedule_push.rs +++ b/backend/windmill-queue/tests/schedule_push.rs @@ -27,7 +27,6 @@ mod schedule_push { is_flow: false, args: None, extra_perms: serde_json::json!({}), - email: "test@windmill.dev".to_string(), permissioned_as: "u/test-user".to_string(), error: None, on_failure: None, @@ -81,7 +80,7 @@ mod schedule_push { permissioned_as: username_to_permissioned_as(&schedule.edited_by), created_by: schedule.edited_by.clone(), script_lang: None, - permissioned_as_email: schedule.email.clone(), + permissioned_as_email: "test@windmill.dev".to_string(), flow_step_id: None, trigger_kind: Some(JobTriggerKind::Schedule.into()), trigger: Some(schedule.path.clone()), @@ -1549,15 +1548,14 @@ mod schedule_push { }); sqlx::query( "INSERT INTO schedule (workspace_id, path, schedule, timezone, edited_by, script_path, - is_flow, enabled, email, permissioned_as, cron_version) - VALUES ($1, $2, $3, 'UTC', $4, $5, false, true, $6, $7, 'v2')", + is_flow, enabled, permissioned_as, cron_version) + VALUES ($1, $2, $3, 'UTC', $4, $5, false, true, $6, 'v2')", ) .bind(&schedule.workspace_id) .bind(&schedule.path) .bind(&schedule.schedule) .bind(&schedule.edited_by) .bind(&schedule.script_path) - .bind(&schedule.email) .bind(&schedule.permissioned_as) .execute(&db) .await?; diff --git a/backend/windmill-types/src/schedule.rs b/backend/windmill-types/src/schedule.rs index 75a48f8798..b74d3186a5 100644 --- a/backend/windmill-types/src/schedule.rs +++ b/backend/windmill-types/src/schedule.rs @@ -17,7 +17,6 @@ pub struct Schedule { pub is_flow: bool, pub args: Option>>, pub extra_perms: serde_json::Value, - pub email: String, pub permissioned_as: String, #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, @@ -65,6 +64,28 @@ impl Schedule { } } +/// A [`Schedule`] plus the address of the identity it runs as, which its own row no longer +/// carries: `permissioned_as` is the identity, and the address is a function of it. +/// +/// The two read paths fill it differently, and deliberately. `get_schedule` derives it, so the +/// response says what the *next run* will resolve to. The workspace export reads the stored +/// column, so a synced file reproduces the row — a principal whose account has since been +/// removed keeps the address the file already had instead of turning into a synthetic +/// `@unknown.windmill.dev`. The column goes when +/// `windmill_common::min_version::MIN_VERSION_DERIVES_SCHEDULE_EMAIL` expires (not a link: this +/// crate is below `windmill-common`, not above it); this field stays, filled by deriving on both +/// paths. +// No `Deserialize`: `Schedule` is flattened in, and serde's flatten buffers through an untagged +// representation that `RawValue` (this type's `args`) cannot be read back from. `FromRow` is +// unaffected — it reads columns by name. +#[derive(Serialize, FromRow, Debug, Clone)] +pub struct ScheduleWithEmail { + #[serde(flatten)] + #[sqlx(flatten)] + pub schedule: Schedule, + pub email: String, +} + pub fn schedule_to_user(path: &str) -> String { format!("schedule-{}", path.replace('/', "-")) }