From 220f5f49722c2eaec0d4f459f76daecd88bcba97 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 2 Aug 2026 22:11:29 +0000 Subject: [PATCH] refactor: drop the app-policy version constant that gated nothing --- ...091932_backfill_app_policy_on_behalf_of.up.sql | 8 ++++---- backend/windmill-api/src/apps.rs | 8 ++++---- backend/windmill-common/src/min_version.rs | 8 -------- docs/app-policy-email-removal.md | 15 +++++++++------ 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/backend/migrations/20260801091932_backfill_app_policy_on_behalf_of.up.sql b/backend/migrations/20260801091932_backfill_app_policy_on_behalf_of.up.sql index f3678edc37..0c77f395bf 100644 --- a/backend/migrations/20260801091932_backfill_app_policy_on_behalf_of.up.sql +++ b/backend/migrations/20260801091932_backfill_app_policy_on_behalf_of.up.sql @@ -2,10 +2,10 @@ -- `policy.on_behalf_of` becomes the authority for an app's identity: the address beside it is -- written through from it on every save, so the two can no longer name different accounts. -- --- The address key is deliberately NOT removed here, and is still written: a replica below --- `MIN_VERSION_DERIVES_APP_POLICY_EMAIL` errors outright when it is absent, which would 400 --- every anonymous and publisher app served by one that has not yet rolled over. Removing the key --- is a follow-up, per docs/app-policy-email-removal.md. +-- The address key is deliberately NOT removed here, and is still written: a replica predating +-- the derive-when-absent fallback errors outright when it is missing, which would 400 every +-- anonymous and publisher app served by one that has not yet rolled over. Removing the key is a +-- follow-up, per docs/app-policy-email-removal.md. -- -- What is left is the backfill: a policy that only ever had the address has no principal to run -- as, so give it one. diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 7292a9234d..25cfc090c9 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -4661,10 +4661,10 @@ async fn app_load_csv_preview() -> Result<()> { /// Resolved uncached: unlike a read, this value is persisted, so a stale cached address would /// stay wrong in the row rather than for the minute the cache lives. /// -/// Written unconditionally, including for versions that could derive it: a replica below -/// `MIN_VERSION_DERIVES_APP_POLICY_EMAIL` fails outright when the key is absent, which would 400 -/// every anonymous and publisher app until a rolling deploy finished. Both the write and the key -/// go together, a release later — see `docs/app-policy-email-removal.md`. +/// Written unconditionally, including for the versions that could derive it instead: a replica +/// predating that fallback fails outright when the key is absent, which would 400 every +/// anonymous and publisher app for the length of a rolling deploy. The write is what holds the +/// key in place — see `docs/app-policy-email-removal.md`. async fn stored_on_behalf_of_email( policy: &Policy, w_id: &str, diff --git a/backend/windmill-common/src/min_version.rs b/backend/windmill-common/src/min_version.rs index 787af12a05..80b3871f3d 100644 --- a/backend/windmill-common/src/min_version.rs +++ b/backend/windmill-common/src/min_version.rs @@ -8,14 +8,6 @@ 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 *requiring* `policy.on_behalf_of_email`: an app policy carrying -/// only a principal executes, deriving the address from it. The key is still written, so a -/// replica below this version keeps finding what it expects. Nothing gates on this at runtime; -/// it exists so `vc()`'s compile-time assert fires once `MIN_KEEP_ALIVE_VERSION` passes it, at -/// which point the key can go — see `docs/app-policy-email-removal.md`. -pub const MIN_VERSION_DERIVES_APP_POLICY_EMAIL: VC = - vc(1, 778, 0, "App policy email derived from on_behalf_of"); 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/docs/app-policy-email-removal.md b/docs/app-policy-email-removal.md index a5a5abd649..40367a6318 100644 --- a/docs/app-policy-email-removal.md +++ b/docs/app-policy-email-removal.md @@ -7,13 +7,16 @@ thing holding it in place. ## The gate -A replica below `MIN_VERSION_DERIVES_APP_POLICY_EMAIL` (`windmill-common/src/min_version.rs`) -*requires* the key: its `get_on_behalf_of` errors outright when the key is absent, so it would -400 every anonymous and publisher app saved without one. A rolling deploy runs both versions at -once, which is why the write stays until no such replica can be live. +`get_on_behalf_of` gained its derive-when-absent fallback in **1.777**. Every replica before that +*requires* the key and errors outright without it, so it would 400 every anonymous and publisher +app saved by a newer one. A rolling deploy runs both versions at once, which is why the write +stays until no replica older than 1.777 can be live — in practice, once `MIN_KEEP_ALIVE_VERSION` +(`windmill-common/src/min_version.rs`) has passed it. -Nothing enforces this at runtime. `vc()`'s compile-time assert fires once `MIN_KEEP_ALIVE_VERSION` -passes that version, and when this constraint stops compiling the gate has been reached. +There is no `MIN_VERSION_*` constant for this and it does not need one: those exist to gate +behavior at runtime or to trip the build when a constraint expires, and nothing here does either. +The key is written unconditionally, so no replica ever meets its absence until someone follows +the steps below. ## Step 1 — stop writing the key