From 6d9edc8c2b01c99339c156dc84def13b0a2205a6 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 1 Feb 2025 09:26:02 +0100 Subject: [PATCH] fix: only start smtp servers if email domain is set --- backend/src/main.rs | 62 ++++++++++++++++++--------------- backend/src/monitor.rs | 3 +- backend/windmill-api/src/lib.rs | 36 ++++++++++++++----- 3 files changed, 62 insertions(+), 39 deletions(-) diff --git a/backend/src/main.rs b/backend/src/main.rs index 70ccb7b72891a..d4459a3371984 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -32,15 +32,15 @@ use windmill_common::{ global_settings::{ BASE_URL_SETTING, BUNFIG_INSTALL_SCOPES_SETTING, CRITICAL_ALERT_MUTE_UI_SETTING, CRITICAL_ERROR_CHANNELS_SETTING, CUSTOM_TAGS_SETTING, DEFAULT_TAGS_PER_WORKSPACE_SETTING, - DEFAULT_TAGS_WORKSPACES_SETTING, ENV_SETTINGS, EXPOSE_DEBUG_METRICS_SETTING, - EXPOSE_METRICS_SETTING, EXTRA_PIP_INDEX_URL_SETTING, HUB_BASE_URL_SETTING, INDEXER_SETTING, - INSTANCE_PYTHON_VERSION_SETTING, JOB_DEFAULT_TIMEOUT_SECS_SETTING, JWT_SECRET_SETTING, - KEEP_JOB_DIR_SETTING, LICENSE_KEY_SETTING, MONITOR_LOGS_ON_OBJECT_STORE_SETTING, - NPM_CONFIG_REGISTRY_SETTING, NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, - PIP_INDEX_URL_SETTING, REQUEST_SIZE_LIMIT_SETTING, - REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, RETENTION_PERIOD_SECS_SETTING, - SAML_METADATA_SETTING, SCIM_TOKEN_SETTING, SMTP_SETTING, TEAMS_SETTING, - TIMEOUT_WAIT_RESULT_SETTING, + DEFAULT_TAGS_WORKSPACES_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, + EXPOSE_DEBUG_METRICS_SETTING, EXPOSE_METRICS_SETTING, EXTRA_PIP_INDEX_URL_SETTING, + HUB_BASE_URL_SETTING, INDEXER_SETTING, INSTANCE_PYTHON_VERSION_SETTING, + JOB_DEFAULT_TIMEOUT_SECS_SETTING, JWT_SECRET_SETTING, KEEP_JOB_DIR_SETTING, + LICENSE_KEY_SETTING, MONITOR_LOGS_ON_OBJECT_STORE_SETTING, NPM_CONFIG_REGISTRY_SETTING, + NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, PIP_INDEX_URL_SETTING, + REQUEST_SIZE_LIMIT_SETTING, REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, + RETENTION_PERIOD_SECS_SETTING, SAML_METADATA_SETTING, SCIM_TOKEN_SETTING, SMTP_SETTING, + TEAMS_SETTING, TIMEOUT_WAIT_RESULT_SETTING, }, scripts::ScriptLang, stats_ee::schedule_stats, @@ -785,11 +785,12 @@ Windmill Community Edition {GIT_VERSION} }, EXPOSE_METRICS_SETTING => { tracing::info!("Metrics setting changed, restarting"); - // we wait a bit randomly to avoid having all servers and workers shutdown at same time - let rd_delay = rand::rng().random_range(0..40); - tokio::time::sleep(Duration::from_secs(rd_delay)).await; - if let Err(e) = tx.send(()) { - tracing::error!(error = %e, "Could not send killpill to server"); + send_delayed_killpill(&tx, 40, "metrics setting change").await; + }, + EMAIL_DOMAIN_SETTING => { + tracing::info!("Email domain setting changed"); + if server_mode { + send_delayed_killpill(&tx, 4, "email domain setting change").await; } }, EXPOSE_DEBUG_METRICS_SETTING => { @@ -799,29 +800,17 @@ Windmill Community Edition {GIT_VERSION} }, OTEL_SETTING => { tracing::info!("OTEL setting changed, restarting"); - // we wait a bit randomly to avoid having all servers and workers shutdown at same time - let rd_delay = rand::rng().random_range(0..4); - tokio::time::sleep(Duration::from_secs(rd_delay)).await; - if let Err(e) = tx.send(()) { - tracing::error!(error = %e, "Could not send killpill"); - } + send_delayed_killpill(&tx, 4, "OTEL setting change").await; }, REQUEST_SIZE_LIMIT_SETTING => { if server_mode { tracing::info!("Request limit size change detected, killing server expecting to be restarted"); - // we wait a bit randomly to avoid having all servers shutdown at same time - let rd_delay = rand::rng().random_range(0..4); - tokio::time::sleep(Duration::from_secs(rd_delay)).await; - if let Err(e) = tx.send(()) { - tracing::error!(error = %e, "Could not send killpill to server"); - } + send_delayed_killpill(&tx, 4, "request size limit change").await; } }, SAML_METADATA_SETTING => { tracing::info!("SAML metadata change detected, killing server expecting to be restarted"); - if let Err(e) = tx.send(()) { - tracing::error!(error = %e, "Could not send killpill to server"); - } + send_delayed_killpill(&tx, 0, "SAML metadata change").await; }, HUB_BASE_URL_SETTING => { if let Err(e) = reload_hub_base_url_setting(&db, server_mode).await { @@ -1094,3 +1083,18 @@ pub async fn run_workers( futures::future::try_join_all(handles).await?; Ok(()) } + +async fn send_delayed_killpill( + tx: &tokio::sync::broadcast::Sender<()>, + max_delay_secs: u64, + context: &str, +) { + // Random delay to avoid all servers/workers shutting down simultaneously + let rd_delay = rand::rng().random_range(0..max_delay_secs); + tracing::info!("Scheduling {context} shutdown in {rd_delay}s"); + tokio::time::sleep(Duration::from_secs(rd_delay)).await; + + if let Err(e) = tx.send(()) { + tracing::error!(error = %e, "Could not send killpill for {context}"); + } +} diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 33488c6356f2c..b77d097ab1713 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -1094,7 +1094,7 @@ pub async fn reload_option_setting_with_tracing( } } -async fn load_value_from_global_settings( +pub async fn load_value_from_global_settings( db: &DB, setting_name: &str, ) -> error::Result> { @@ -1107,6 +1107,7 @@ async fn load_value_from_global_settings( .map(|x| x.value); Ok(r) } + pub async fn reload_option_setting( db: &DB, setting_name: &str, diff --git a/backend/windmill-api/src/lib.rs b/backend/windmill-api/src/lib.rs index 7dfba6ce55377..7ab057292fac0 100644 --- a/backend/windmill-api/src/lib.rs +++ b/backend/windmill-api/src/lib.rs @@ -34,6 +34,8 @@ use http::HeaderValue; use reqwest::Client; #[cfg(feature = "oauth2")] use std::collections::HashMap; +use windmill_common::global_settings::load_value_from_global_settings; +use windmill_common::global_settings::EMAIL_DOMAIN_SETTING; use windmill_common::worker::HUB_CACHE_DIR; use std::fs::DirBuilder; @@ -251,16 +253,32 @@ pub async fn run_server( #[cfg(feature = "embedding")] load_embeddings_db(&db); - #[cfg(feature = "smtp")] + let mut start_smtp_server = false; + if let Some(smtp_settings) = + load_value_from_global_settings(&db, EMAIL_DOMAIN_SETTING).await? { - let smtp_server = Arc::new(SmtpServer { - db: db.clone(), - user_db: user_db, - auth_cache: auth_cache.clone(), - base_internal_url: base_internal_url.clone(), - }); - if let Err(err) = smtp_server.start_listener_thread(addr).await { - tracing::error!("Error starting SMTP server: {err:#}"); + if smtp_settings.as_str().unwrap_or("") != "" { + start_smtp_server = true; + } + } + if !start_smtp_server { + tracing::info!("SMTP server not started because email domain is not set"); + } else { + #[cfg(feature = "smtp")] + { + let smtp_server = Arc::new(SmtpServer { + db: db.clone(), + user_db: user_db, + auth_cache: auth_cache.clone(), + base_internal_url: base_internal_url.clone(), + }); + if let Err(err) = smtp_server.start_listener_thread(addr).await { + tracing::error!("Error starting SMTP server: {err:#}"); + } + } + #[cfg(not(feature = "smtp"))] + { + tracing::info!("SMTP server not started because SMTP feature is not enabled"); } } }