From 9a67a8119422c60d129c5b4602b033fc71b8d204 Mon Sep 17 00:00:00 2001 From: Nadav Gov-Ari Date: Wed, 21 Jan 2026 14:19:16 -0500 Subject: [PATCH 1/4] Catch empty AZ env vars --- quickwit/quickwit-config/src/node_config/serialize.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/quickwit/quickwit-config/src/node_config/serialize.rs b/quickwit/quickwit-config/src/node_config/serialize.rs index 7d3b5f2c64b..4a15b5ecad0 100644 --- a/quickwit/quickwit-config/src/node_config/serialize.rs +++ b/quickwit/quickwit-config/src/node_config/serialize.rs @@ -224,7 +224,11 @@ impl NodeConfigBuilder { env_vars: &HashMap, ) -> anyhow::Result { let node_id = self.node_id.resolve(env_vars).map(NodeId::new)?; - let availability_zone = self.availability_zone.resolve_optional(env_vars)?; + let availability_zone = self + .availability_zone + .resolve_optional(env_vars)? + // if the env var is empty, it'll return Some(""), and we don't want that. + .filter(|str| !str.is_empty()); let enabled_services = self .enabled_services From a1c0a9251110f711246c30eb742fbd3601a92d1d Mon Sep 17 00:00:00 2001 From: Nadav Gov-Ari Date: Thu, 22 Jan 2026 14:37:56 -0500 Subject: [PATCH 2/4] move into resolve_optional and add test --- quickwit/quickwit-config/src/config_value.rs | 12 ++++++++++-- .../quickwit-config/src/node_config/serialize.rs | 4 +--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/quickwit/quickwit-config/src/config_value.rs b/quickwit/quickwit-config/src/config_value.rs index c0620359a65..ce6632dfe24 100644 --- a/quickwit/quickwit-config/src/config_value.rs +++ b/quickwit/quickwit-config/src/config_value.rs @@ -68,7 +68,7 @@ where // QW env vars take precedence over the config file values. if E > QW_NONE && let Some(env_var_key) = QW_ENV_VARS.get(&E) - && let Some(env_var_value) = env_vars.get(*env_var_key) + && let Some(env_var_value) = env_vars.get(*env_var_key).filter(|val| !val.is_empty()) { let value = env_var_value.parse::().map_err(|error| { anyhow::anyhow!( @@ -118,7 +118,7 @@ where T: Deserialize<'de> mod tests { use super::*; use crate::qw_env_vars::{ - QW_CLUSTER_ID, QW_GOSSIP_LISTEN_PORT, QW_NODE_ID, QW_REST_LISTEN_PORT, + QW_CLUSTER_ID, QW_GOSSIP_LISTEN_PORT, QW_NODE_ID, QW_REST_LISTEN_PORT, QW_AVAILABILITY_ZONE, }; #[test] @@ -191,6 +191,14 @@ mod tests { rest_listen_port.resolve(&env_vars).unwrap_err(); } + #[test] + fn test_config_value_resolve_optional_empty_string() { + let mut env_vars = HashMap::new(); + env_vars.insert(String::from("QW_AVAILABILITY_ZONE"), String::from("")); + let az = ConfigValue::::none(); + assert_eq!(az.resolve_optional(&env_vars).unwrap(), None); + } + #[test] fn test_config_value_deserialize() { fn default_cluster_id() -> ConfigValue { diff --git a/quickwit/quickwit-config/src/node_config/serialize.rs b/quickwit/quickwit-config/src/node_config/serialize.rs index 4a15b5ecad0..d6c2d609304 100644 --- a/quickwit/quickwit-config/src/node_config/serialize.rs +++ b/quickwit/quickwit-config/src/node_config/serialize.rs @@ -226,9 +226,7 @@ impl NodeConfigBuilder { let node_id = self.node_id.resolve(env_vars).map(NodeId::new)?; let availability_zone = self .availability_zone - .resolve_optional(env_vars)? - // if the env var is empty, it'll return Some(""), and we don't want that. - .filter(|str| !str.is_empty()); + .resolve_optional(env_vars)?; let enabled_services = self .enabled_services From 88ddef10058c48a895dbd45d7e54efbe22d641c9 Mon Sep 17 00:00:00 2001 From: Nadav Gov-Ari Date: Fri, 23 Jan 2026 15:49:46 -0500 Subject: [PATCH 3/4] PR suggestion --- quickwit/quickwit-config/src/config_value.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/quickwit/quickwit-config/src/config_value.rs b/quickwit/quickwit-config/src/config_value.rs index ce6632dfe24..7ca08534e40 100644 --- a/quickwit/quickwit-config/src/config_value.rs +++ b/quickwit/quickwit-config/src/config_value.rs @@ -18,7 +18,7 @@ use std::{any, fmt}; use anyhow::{self, Context}; use serde::{Deserialize, Deserializer}; - +use tracing::log::warn; use crate::qw_env_vars::{QW_ENV_VARS, QW_NONE}; #[derive(Debug, Clone, Eq, PartialEq)] @@ -68,7 +68,14 @@ where // QW env vars take precedence over the config file values. if E > QW_NONE && let Some(env_var_key) = QW_ENV_VARS.get(&E) - && let Some(env_var_value) = env_vars.get(*env_var_key).filter(|val| !val.is_empty()) + && let Some(env_var_value) = env_vars.get(*env_var_key).filter(|val| { + if val.is_empty() { + warn!("environment variable `{}` is set but value is empty", *env_var_key); + false + } else { + true + } + }) { let value = env_var_value.parse::().map_err(|error| { anyhow::anyhow!( From 6448ff3af3f632b67a9a925d9af139af52d46d35 Mon Sep 17 00:00:00 2001 From: Adrien Guillo Date: Fri, 23 Jan 2026 16:20:38 -0500 Subject: [PATCH 4/4] Fix fmt, lints, etc. --- quickwit/quickwit-config/src/config_value.rs | 22 +++++++++++-------- .../src/node_config/serialize.rs | 4 +--- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/quickwit/quickwit-config/src/config_value.rs b/quickwit/quickwit-config/src/config_value.rs index 7ca08534e40..36a35e2f11b 100644 --- a/quickwit/quickwit-config/src/config_value.rs +++ b/quickwit/quickwit-config/src/config_value.rs @@ -19,6 +19,7 @@ use std::{any, fmt}; use anyhow::{self, Context}; use serde::{Deserialize, Deserializer}; use tracing::log::warn; + use crate::qw_env_vars::{QW_ENV_VARS, QW_NONE}; #[derive(Debug, Clone, Eq, PartialEq)] @@ -69,12 +70,15 @@ where if E > QW_NONE && let Some(env_var_key) = QW_ENV_VARS.get(&E) && let Some(env_var_value) = env_vars.get(*env_var_key).filter(|val| { - if val.is_empty() { - warn!("environment variable `{}` is set but value is empty", *env_var_key); - false - } else { - true - } + if val.is_empty() { + warn!( + "environment variable `{}` is set but value is empty", + *env_var_key + ); + false + } else { + true + } }) { let value = env_var_value.parse::().map_err(|error| { @@ -125,7 +129,7 @@ where T: Deserialize<'de> mod tests { use super::*; use crate::qw_env_vars::{ - QW_CLUSTER_ID, QW_GOSSIP_LISTEN_PORT, QW_NODE_ID, QW_REST_LISTEN_PORT, QW_AVAILABILITY_ZONE, + QW_AVAILABILITY_ZONE, QW_CLUSTER_ID, QW_GOSSIP_LISTEN_PORT, QW_NODE_ID, QW_REST_LISTEN_PORT, }; #[test] @@ -201,9 +205,9 @@ mod tests { #[test] fn test_config_value_resolve_optional_empty_string() { let mut env_vars = HashMap::new(); - env_vars.insert(String::from("QW_AVAILABILITY_ZONE"), String::from("")); + env_vars.insert("QW_AVAILABILITY_ZONE".to_string(), "".to_string()); let az = ConfigValue::::none(); - assert_eq!(az.resolve_optional(&env_vars).unwrap(), None); + assert!(az.resolve_optional(&env_vars).unwrap().is_none()); } #[test] diff --git a/quickwit/quickwit-config/src/node_config/serialize.rs b/quickwit/quickwit-config/src/node_config/serialize.rs index d6c2d609304..7d3b5f2c64b 100644 --- a/quickwit/quickwit-config/src/node_config/serialize.rs +++ b/quickwit/quickwit-config/src/node_config/serialize.rs @@ -224,9 +224,7 @@ impl NodeConfigBuilder { env_vars: &HashMap, ) -> anyhow::Result { let node_id = self.node_id.resolve(env_vars).map(NodeId::new)?; - let availability_zone = self - .availability_zone - .resolve_optional(env_vars)?; + let availability_zone = self.availability_zone.resolve_optional(env_vars)?; let enabled_services = self .enabled_services