From 8f46066a6478a48411e00f5c4e1f9b9fdb49c1fc Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 31 Aug 2025 12:19:59 +0000 Subject: [PATCH] feat(perf): add 300-second local cache for variable crypt retrieval (#6483) * feat: add 60-second cache for workspace key retrieval This implements a cache with 60-second staleness for the get_workspace_key function to reduce database queries for workspace encryption keys. The cache follows the same pattern as the existing CUSTOM_ENVS_CACHE but with a shorter expiration time. Requested by @rubenfiszel Co-authored-by: windmill-internal-app[bot] * Extend cache staleness from 60 to 300 seconds * feat: add cache invalidation notifications for workspace keys Add PostgreSQL LISTEN/NOTIFY mechanism to invalidate workspace key cache across all servers and workers when workspace keys change. - Add database migration with trigger function for workspace_key changes - Add notification handler in main.rs to remove from WORKSPACE_KEY_CACHE - Follow same pattern as existing workspace environment cache invalidation - Ensures distributed cache consistency for workspace encryption keys Co-authored-by: Ruben Fiszel * finish --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: windmill-internal-app[bot] Co-authored-by: Ruben Fiszel Co-authored-by: Ruben Fiszel --- ...0250827152300_workspace_key_cache.down.sql | 4 ++ .../20250827152300_workspace_key_cache.up.sql | 19 ++++++++++ backend/src/main.rs | 6 +++ backend/windmill-api/src/workspaces.rs | 12 +++++- backend/windmill-common/src/variables.rs | 38 ++++++++++++++++--- 5 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 backend/migrations/20250827152300_workspace_key_cache.down.sql create mode 100644 backend/migrations/20250827152300_workspace_key_cache.up.sql diff --git a/backend/migrations/20250827152300_workspace_key_cache.down.sql b/backend/migrations/20250827152300_workspace_key_cache.down.sql new file mode 100644 index 0000000000..23f95616ff --- /dev/null +++ b/backend/migrations/20250827152300_workspace_key_cache.down.sql @@ -0,0 +1,4 @@ +-- Remove workspace key cache invalidation trigger + +DROP TRIGGER workspace_key_change_trigger ON workspace_key; +DROP FUNCTION notify_workspace_key_change(); \ No newline at end of file diff --git a/backend/migrations/20250827152300_workspace_key_cache.up.sql b/backend/migrations/20250827152300_workspace_key_cache.up.sql new file mode 100644 index 0000000000..13f374625f --- /dev/null +++ b/backend/migrations/20250827152300_workspace_key_cache.up.sql @@ -0,0 +1,19 @@ +-- Add workspace key cache invalidation trigger + +CREATE OR REPLACE FUNCTION notify_workspace_key_change() +RETURNS TRIGGER AS $$ +BEGIN + IF TG_OP = 'DELETE' THEN + PERFORM pg_notify('notify_workspace_key_change', OLD.workspace_id); + RETURN OLD; + ELSE + PERFORM pg_notify('notify_workspace_key_change', NEW.workspace_id); + RETURN NEW; + END IF; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER workspace_key_change_trigger +AFTER INSERT OR UPDATE OF key OR DELETE ON workspace_key +FOR EACH ROW +EXECUTE FUNCTION notify_workspace_key_change(); \ No newline at end of file diff --git a/backend/src/main.rs b/backend/src/main.rs index be0b86ec46..1dce88b3dc 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -854,6 +854,11 @@ Windmill Community Edition {GIT_VERSION} tracing::info!("Workspace envs change detected, invalidating workspace envs cache: {}", workspace_id); windmill_common::variables::CUSTOM_ENVS_CACHE.remove(workspace_id); }, + "notify_workspace_key_change" => { + let workspace_id = n.payload(); + tracing::info!("Workspace key change detected, invalidating workspace key cache: {}", workspace_id); + windmill_common::variables::WORKSPACE_CRYPT_CACHE.remove(workspace_id); + }, "notify_workspace_premium_change" => { let workspace_id = n.payload(); tracing::info!("Workspace premium change detected, invalidating workspace premium cache: {}", workspace_id); @@ -1268,6 +1273,7 @@ async fn listen_pg(url: &str) -> Option { "notify_global_setting_change", "notify_webhook_change", "notify_workspace_envs_change", + "notify_workspace_key_change", "notify_runnable_version_change", "notify_token_invalidation", ]; diff --git a/backend/windmill-api/src/workspaces.rs b/backend/windmill-api/src/workspaces.rs index 9e3c7dc25e..fb0ea74f26 100644 --- a/backend/windmill-api/src/workspaces.rs +++ b/backend/windmill-api/src/workspaces.rs @@ -35,7 +35,7 @@ use windmill_audit::ActionKind; use windmill_common::db::UserDB; use windmill_common::s3_helpers::LargeFileStorage; use windmill_common::users::username_to_permissioned_as; -use windmill_common::variables::{build_crypt, decrypt, encrypt}; +use windmill_common::variables::{build_crypt, decrypt, encrypt, WORKSPACE_CRYPT_CACHE}; use windmill_common::worker::{to_raw_value, CLOUD_HOSTED}; #[cfg(feature = "enterprise")] use windmill_common::workspaces::GitRepositorySettings; @@ -1928,6 +1928,8 @@ async fn set_encryption_key( .execute(&db) .await?; + WORKSPACE_CRYPT_CACHE.remove(w_id.as_str()); + if !request.skip_reencrypt.unwrap_or(false) { let new_encryption_key = build_crypt(&db, w_id.as_str()).await?; @@ -1950,7 +1952,13 @@ async fn set_encryption_key( if !variable.is_secret { continue; } - let decrypted_value = decrypt(&previous_encryption_key, variable.value)?; + let decrypted_value = + decrypt(&previous_encryption_key, variable.value).map_err(|e| { + Error::internal_err(format!( + "Error decrypting variable {}: {}", + variable.path, e + )) + })?; let new_encrypted_value = encrypt(&new_encryption_key, decrypted_value.as_str()); sqlx::query!( "UPDATE variable SET value = $1 WHERE workspace_id = $2 AND path = $3", diff --git a/backend/windmill-common/src/variables.rs b/backend/windmill-common/src/variables.rs index 740c82366a..a1ab8e45cd 100644 --- a/backend/windmill-common/src/variables.rs +++ b/backend/windmill-common/src/variables.rs @@ -78,13 +78,38 @@ pub struct CreateVariable { } pub async fn build_crypt(db: &DB, w_id: &str) -> crate::error::Result { - let key = get_workspace_key(w_id, db).await?; - let crypt_key = if let Some(ref salt) = SECRET_SALT.as_ref() { - format!("{}{}", key, salt) + // Check cache first (300-second staleness) + let cached_key_o = WORKSPACE_CRYPT_CACHE.get(w_id).and_then(|(ts, key)| { + if ts > chrono::Utc::now().timestamp() - 300 { + Some(key) + } else { + None + } + }); + + let crypt = if let Some(cached_key) = cached_key_o { + cached_key } else { - key + let key = get_workspace_key(w_id, db).await?; + tracing::info!( + "crypt for workspace {} with key {} expired, refetching", + w_id, + key + ); + let crypt_key = if let Some(ref salt) = SECRET_SALT.as_ref() { + format!("{}{}", key, salt) + } else { + key + }; + let ncrypt = magic_crypt::new_magic_crypt!(crypt_key, 256); + WORKSPACE_CRYPT_CACHE.insert( + w_id.to_string(), + (chrono::Utc::now().timestamp(), ncrypt.clone()), + ); + ncrypt }; - Ok(magic_crypt::new_magic_crypt!(crypt_key, 256)) + + Ok(crypt) } pub async fn build_crypt_with_key_suffix( @@ -110,6 +135,7 @@ pub async fn get_workspace_key(w_id: &str, db: &DB) -> crate::error::Result)> = Cache::new(100); + pub static ref WORKSPACE_CRYPT_CACHE: Cache = Cache::new(1000); + } pub async fn get_reserved_variables(