This is an automated email from the ASF dual-hosted git repository.
hubcio pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iggy.git
The following commit(s) were added to refs/heads/master by this push:
new d1cb5aee7 fix(configs): check server env vars once and refuse boot
only in debug (#4200)
d1cb5aee7 is described below
commit d1cb5aee7b00df9ae7f152757cd23bec45087968
Author: Hubert Gruszecki <[email protected]>
AuthorDate: Mon Sep 21 13:15:52 2026 +0200
fix(configs): check server env vars once and refuse boot only in debug
(#4200)
---
core/configs/src/configs_impl/file_provider.rs | 101 ++++++++++++++++-----
.../configs/src/configs_impl/typed_env_provider.rs | 100 +++++++++++++-------
core/configs/src/server_config/server.rs | 36 +++++++-
gateways/kafka/src/bridge/config.rs | 4 +-
gateways/kafka/src/main.rs | 10 +-
5 files changed, 185 insertions(+), 66 deletions(-)
diff --git a/core/configs/src/configs_impl/file_provider.rs
b/core/configs/src/configs_impl/file_provider.rs
index f182f2cd2..851000303 100644
--- a/core/configs/src/configs_impl/file_provider.rs
+++ b/core/configs/src/configs_impl/file_provider.rs
@@ -127,7 +127,9 @@ impl<P: Provider> FileConfigProvider<P> {
self
}
- fn reject_unknown_env_names(&self) -> Result<(), ConfigurationError> {
+ /// Debug builds refuse to boot on an unknown name, so CI and local runs
+ /// catch a stray or misspelled variable. Release builds warn and ignore
it.
+ fn check_unknown_env_names(&self) -> Result<(), ConfigurationError> {
let Some(known) = &self.known_env_names else {
return Ok(());
};
@@ -137,15 +139,24 @@ impl<P: Provider> FileConfigProvider<P> {
known,
self.allowed_env_prefixes,
);
- for name in &unknown {
- eprintln!("Unknown configuration environment variable '{name}'.
Unset it to boot.");
+ if unknown.is_empty() {
+ return Ok(());
}
- let rejected = !unknown.is_empty();
- if rejected {
- Err(ConfigurationError::InvalidConfigurationValue)
+ // Config load runs before the logger is configured, so a `warn!`
record
+ // can be filtered by `RUST_LOG` or lost when boot fails before
`late_init`.
+ let refuse = cfg!(debug_assertions);
+ let remedy = if refuse {
+ "Unset it to boot."
} else {
- Ok(())
+ "It will be ignored."
+ };
+ for name in &unknown {
+ eprintln!("Unknown configuration environment variable '{name}'.
{remedy}");
}
+ if refuse {
+ return Err(ConfigurationError::InvalidConfigurationValue);
+ }
+ Ok(())
}
fn reject_relocated_keys(&self) -> Result<(), ConfigurationError> {
@@ -194,7 +205,7 @@ impl<P: Provider + Clone> ConfigProvider for
FileConfigProvider<P> {
// below is just as silent about a key no field reads, and the
// pure-env container never touches the file branch at all.
self.reject_relocated_keys()?;
- self.reject_unknown_env_names()?;
+ self.check_unknown_env_names()?;
// Start with the default configuration if provided
let mut config_builder = Figment::new();
@@ -429,6 +440,7 @@ mod tests {
"IGGY_CONNECTORS_STATE_PATH",
"IGGY_MCP_CONFIG_PATH",
"IGGY_MCP_TRANSPORT",
+ "IGGY_KAFKA_BIND_ADDR",
"IGGY_HOME",
"IGGY_USERNAME",
"IGGY_PASSWORD",
@@ -457,17 +469,60 @@ mod tests {
}
/// `main.rs` loads a `.env` through `dotenvy` before `load_config` runs,
and
- /// `dotenvy` injects into the process environment that
`reject_unknown_env_names`
+ /// `dotenvy` injects into the process environment that
`check_unknown_env_names`
/// scans with `env::vars_os()`. So the fence does not need a shared
container
/// or a shared `env_file`: a `.env` in the working directory is enough.
- ///
- /// Mutates the process environment, so it must not run beside another test
- /// that reads it.
#[test]
- #[serial_test::serial]
fn
given_a_dotenv_with_a_connectors_variable_when_loading_then_the_server_should_boot()
{
- // SAFETY: single-threaded assertion over a variable no other test
reads.
- unsafe { std::env::set_var("IGGY_CONNECTORS_CONFIG_PATH",
"/etc/iggy/connectors.toml") };
+ let unknown =
server_unknown_env_names(&["IGGY_CONNECTORS_CONFIG_PATH"]);
+
+ assert!(
+ unknown.is_empty(),
+ "a .env naming the connectors runtime's own config path refuses
server boot, with no opt-out and a message that names no remedy"
+ );
+ }
+
+ /// The server reads these variables outside its config, so the boot check
+ /// must accept them. Without the `SERVER_PROCESS_ENV_VARS` chain in
+ /// `ServerConfig::config_provider`, a debug build refuses to boot.
+ #[test]
+ fn
given_the_server_process_variables_when_checking_then_the_server_should_boot() {
+ let unknown =
+
server_unknown_env_names(crate::server_config::server::SERVER_PROCESS_ENV_VARS);
+
+ assert!(
+ unknown.is_empty(),
+ "the boot check must accept every variable the server reads
outside its config, got: {unknown:?}"
+ );
+ }
+
+ /// Runs the boot check's filter over `candidates` with the real server
+ /// wiring, and without reading the ambient environment.
+ fn server_unknown_env_names(candidates: &[&str]) -> Vec<String> {
+ let provider =
+
crate::server_config::server::ServerConfig::config_provider("nonexistent-config.toml");
+ let known = provider
+ .known_env_names
+ .as_ref()
+ .expect("the server provider declares its known env names");
+
+ unknown_env_names(
+ names(candidates).into_iter(),
+ provider.env_prefix,
+ known,
+ provider.allowed_env_prefixes,
+ )
+ }
+
+ #[test]
+ #[serial_test::serial]
+ fn
given_an_unknown_env_var_when_checking_then_only_debug_builds_should_refuse() {
+ const PREFIX: &str = "IGGY_FILE_PROVIDER_TEST_";
+ const UNKNOWN: &str = "IGGY_FILE_PROVIDER_TEST_UNKNOWN";
+ // SAFETY: the race is process-wide, not per key: `set_var` is unsound
+ // against any concurrent environment access. `serial_test::serial` on
+ // this test is what prevents that.
+ unsafe { std::env::set_var(UNKNOWN, "1") };
let provider = FileConfigProvider::new(
"nonexistent-config.toml".to_string(),
@@ -475,17 +530,17 @@ mod tests {
false,
None,
)
- .with_relocated_keys("IGGY_", &[])
-
.with_known_env_names(crate::server_config::server::SERVER_PROCESS_ENV_VARS.to_vec())
-
.with_allowed_env_prefixes(crate::server_config::server::SERVER_ALLOWED_ENV_PREFIXES);
- let rejected = provider.reject_unknown_env_names();
+ .with_relocated_keys(PREFIX, &[])
+ .with_known_env_names(Vec::new());
+ let checked = provider.check_unknown_env_names();
// SAFETY: paired with the set above.
- unsafe { std::env::remove_var("IGGY_CONNECTORS_CONFIG_PATH") };
+ unsafe { std::env::remove_var(UNKNOWN) };
- assert!(
- rejected.is_ok(),
- "a .env naming the connectors runtime's own config path refuses
server boot, with no opt-out and a message that names no remedy"
+ assert_eq!(
+ checked.is_err(),
+ cfg!(debug_assertions),
+ "an unknown variable must refuse boot in debug builds and only
warn in release builds"
);
}
diff --git a/core/configs/src/configs_impl/typed_env_provider.rs
b/core/configs/src/configs_impl/typed_env_provider.rs
index 5225f6a8a..b6be617da 100644
--- a/core/configs/src/configs_impl/typed_env_provider.rs
+++ b/core/configs/src/configs_impl/typed_env_provider.rs
@@ -43,39 +43,19 @@ enum WarningContext<'a> {
ConnectorConfig(&'a str),
}
-/// Environment variables starting with IGGY_ that are NOT config values.
-/// These are used for test control, CI, CLI behavior, config file paths, etc.
+/// `IGGY_` variables that are NOT config values: the config file and dotenv
+/// paths the connectors runtime and the MCP server read before their config
+/// loads.
const IGNORED_ENV_VARS: &[&str] = &[
- "IGGY_CI_BUILD",
- // Test-harness knob: overrides the default cluster size the integration
- // harness builds; leaks to spawned servers via the IGGY_ env forwarding.
- "IGGY_TEST_CLUSTER_NODES",
- "IGGY_CONFIG_PATH",
"IGGY_CONNECTORS_CONFIG_PATH",
+ "IGGY_CONNECTORS_ENV_PATH",
"IGGY_MCP_CONFIG_PATH",
- "IGGY_ROOT_PASSWORD",
- "IGGY_ROOT_USERNAME",
- // Tunes per-shard io_uring SQ/CQ capacity; read directly by
- // `server_common::executor::create_shard_executor` (see that fn for
rationale).
- "IGGY_SHARD_RUNTIME_CAPACITY",
- "IGGY_TEST_CLEANUP_DISABLED",
- "IGGY_TEST_VERBOSE",
+ "IGGY_MCP_ENV_PATH",
];
/// Prefixes for env vars handled by separate providers with runtime prefixes.
/// The main config provider skips these; each sub-provider validates its own
vars.
-///
-/// `IGGY_KAFKA_` (`gateways/kafka/src/main.rs`) parses its own nine vars by
hand rather than via
-/// `#[derive(ConfigEnv)]`, so it needs an entry here the same way the
connector prefixes do -
-/// without it, `iggy-server` (and `cargo test -p integration`, which forwards
`IGGY_*` to spawned
-/// servers) `debug_assert!`s on the first `IGGY_KAFKA_*` var it sees. This
trades away the
-/// typo-detection this provider gives derived configs: an `IGGY_KAFKA_` typo
now silently no-ops
-/// instead of surfacing here.
-const DELEGATED_ENV_VAR_PREFIXES: &[&str] = &[
- "IGGY_CONNECTORS_SINK_",
- "IGGY_CONNECTORS_SOURCE_",
- "IGGY_KAFKA_",
-];
+const DELEGATED_ENV_VAR_PREFIXES: &[&str] = &["IGGY_CONNECTORS_SINK_",
"IGGY_CONNECTORS_SOURCE_"];
type ProfileMap = FigmentMap<Profile, Dict>;
@@ -92,6 +72,7 @@ type ProfileMap = FigmentMap<Profile, Dict>;
pub struct TypedEnvProvider<T: ConfigEnvMappings> {
prefix: String,
secret_keys: Vec<String>,
+ check_unknown_env_vars: bool,
_phantom: PhantomData<T>,
}
@@ -105,6 +86,7 @@ impl<T: ConfigEnvMappings> TypedEnvProvider<T> {
Self {
prefix: prefix.to_string(),
secret_keys: secret_keys.iter().map(|s| s.to_string()).collect(),
+ check_unknown_env_vars: true,
_phantom: PhantomData,
}
}
@@ -141,26 +123,42 @@ impl<T: ConfigEnvMappings> TypedEnvProvider<T> {
Self {
prefix: prefix.to_string(),
secret_keys,
+ check_unknown_env_vars: true,
_phantom: PhantomData,
}
}
+ /// Skip the unknown-variable scan in [`Self::deserialize`] and
+ /// [`Self::deserialize_with_runtime_prefix`].
+ ///
+ /// For a loader that checks every name itself: a second check with its
+ /// own list would flag names that loader accepts.
+ pub fn without_unknown_env_var_check(mut self) -> Self {
+ self.check_unknown_env_vars = false;
+ self
+ }
+
/// Deserialize with runtime prefix prepended to each mapping's env_name.
///
/// Unlike `deserialize()`, this method prepends `self.prefix` to each
mapping's
/// env_name, allowing for dynamic prefix construction at runtime.
pub fn deserialize_with_runtime_prefix(&self) -> Result<ProfileMap,
ConfigurationError> {
-
self.warn_unknown_env_vars_inner(WarningContext::ConnectorConfig(&self.prefix));
+ if self.check_unknown_env_vars {
+
self.warn_unknown_env_vars_inner(WarningContext::ConnectorConfig(&self.prefix));
+ }
self.deserialize_inner(EnvNameResolution::PrependPrefix(&self.prefix))
}
/// Deserialize environment variables into a configuration profile map.
///
/// This method:
- /// 1. Validates that all env vars with the prefix are known (warns on
unknown)
+ /// 1. Validates that all env vars with the prefix are known (warns on
unknown),
+ /// unless [`Self::without_unknown_env_var_check`] turned that off
/// 2. Iterates over compile-time generated mappings and applies set values
pub fn deserialize(&self) -> Result<ProfileMap, ConfigurationError> {
- self.warn_unknown_env_vars_inner(WarningContext::MainConfig);
+ if self.check_unknown_env_vars {
+ self.warn_unknown_env_vars_inner(WarningContext::MainConfig);
+ }
self.deserialize_inner(EnvNameResolution::Direct)
}
@@ -333,11 +331,7 @@ impl<T: ConfigEnvMappings> TypedEnvProvider<T> {
fn warn_unknown_var(unknown_var: &str, suggestions: &[String]) {
if suggestions.is_empty() {
- warn!(
- "Unknown environment variable '{}' will be ignored. \
- Use --list-env-vars to see all valid environment variables.",
- unknown_var
- );
+ warn!("Unknown environment variable '{unknown_var}' will be
ignored.");
} else {
warn!(
"Unknown environment variable '{}' will be ignored. Similar
variables: {}?",
@@ -622,4 +616,42 @@ mod tests {
panic!("count should be u64");
}
}
+
+ /// A sibling binary's path arrives here through a shared environment, or
+ /// through one `.env` that every binary loads. Neither path is a config
+ /// value, and a debug build refuses to boot on an unknown name, so the
scan
+ /// has to skip all four. The list check holds in both build profiles, and
+ /// the scan adds the `debug_assert!` path in a debug build.
+ #[test]
+ #[serial_test::serial]
+ fn ignored_env_vars_are_skipped_by_the_unknown_variable_scan() {
+ for name in [
+ "IGGY_CONNECTORS_CONFIG_PATH",
+ "IGGY_CONNECTORS_ENV_PATH",
+ "IGGY_MCP_CONFIG_PATH",
+ "IGGY_MCP_ENV_PATH",
+ ] {
+ assert!(
+ IGNORED_ENV_VARS.contains(&name),
+ "{name} is read by a sibling binary before its config loads,
so the scan must skip it"
+ );
+ }
+
+ for name in IGNORED_ENV_VARS {
+ // SAFETY: the race is process-wide, not per key: `set_var` is
unsound
+ // against any concurrent environment access.
`serial_test::serial` on
+ // this test is what prevents that.
+ unsafe { env::set_var(name, "/etc/iggy/ignored") };
+ }
+
+ for prefix in ["IGGY_CONNECTORS_", "IGGY_MCP_"] {
+ TypedEnvProvider::<TestConfig>::new(prefix, &[])
+ .warn_unknown_env_vars_inner(WarningContext::MainConfig);
+ }
+
+ for name in IGNORED_ENV_VARS {
+ // SAFETY: paired with the set above.
+ unsafe { env::remove_var(name) };
+ }
+ }
}
diff --git a/core/configs/src/server_config/server.rs
b/core/configs/src/server_config/server.rs
index 20879d993..69985923f 100644
--- a/core/configs/src/server_config/server.rs
+++ b/core/configs/src/server_config/server.rs
@@ -66,7 +66,8 @@ pub const SERVER_PROCESS_ENV_VARS: &[&str] = &[
"IGGY_PASSWORD",
];
-pub(crate) const SERVER_ALLOWED_ENV_PREFIXES: &[&str] = &["IGGY_CONNECTORS_",
"IGGY_MCP_"];
+pub(crate) const SERVER_ALLOWED_ENV_PREFIXES: &[&str] =
+ &["IGGY_CONNECTORS_", "IGGY_KAFKA_", "IGGY_MCP_"];
const DEFAULT_CONFIG_PATH: &str = "core/server/config.toml";
@@ -287,7 +288,10 @@ pub struct ServerConfigEnvProvider {
impl Default for ServerConfigEnvProvider {
fn default() -> Self {
Self {
- provider: TypedEnvProvider::from_config(ServerConfig::ENV_PREFIX),
+ // `ServerConfig::config_provider` checks every `IGGY_` name before
+ // this provider runs.
+ provider: TypedEnvProvider::from_config(ServerConfig::ENV_PREFIX)
+ .without_unknown_env_var_check(),
}
}
}
@@ -513,4 +517,32 @@ mod tests {
"expected at least one IGGY_MESSAGE_BUS_* env var, got: {names:?}"
);
}
+
+ #[test]
+ #[serial_test::serial]
+ fn env_provider_accepts_server_process_env_vars() {
+ for name in SERVER_PROCESS_ENV_VARS {
+ // SAFETY: the race is process-wide, not per key: `set_var` is
unsound
+ // against any concurrent environment access.
`serial_test::serial` on
+ // this test is what prevents that.
+ unsafe { env::set_var(name, "1") };
+ }
+
+ let data = ServerConfigEnvProvider::default().data();
+
+ for name in SERVER_PROCESS_ENV_VARS {
+ // SAFETY: paired with the set above.
+ unsafe { env::remove_var(name) };
+ }
+
+ // The provider holds no scan of its own, so the typed provider's
+ // debug_assert! stays quiet. A panic above is one failure this test
+ // guards, and one of these names reaching the map is the other.
+ let data = data.expect("the server env provider must accept every
process variable");
+ let profile = data.get(&Profile::default()).expect("no default
profile");
+ assert!(
+ profile.is_empty(),
+ "none of these variables is a config value, so none of them may
reach the map: {profile:?}"
+ );
+ }
}
diff --git a/gateways/kafka/src/bridge/config.rs
b/gateways/kafka/src/bridge/config.rs
index 41de1fcc3..63266f480 100644
--- a/gateways/kafka/src/bridge/config.rs
+++ b/gateways/kafka/src/bridge/config.rs
@@ -49,8 +49,8 @@ impl IggyBridgeConfig {
/// `KNOWN_KAFKA_ENV_VARS`, not one merged copy - a var added only here is
already
/// recognized there with no corresponding edit needed, and vice versa. A
var this module
/// reads still has to be listed *somewhere* the guard checks, or a typo
silently no-ops
- /// instead of surfacing (`IGGY_KAFKA_` is a `DELEGATED_ENV_VAR_PREFIXES`
entry in
- /// `core/configs`, so the central provider's own typo-detection doesn't
cover this namespace
+ /// instead of surfacing (`IGGY_KAFKA_` is a `SERVER_ALLOWED_ENV_PREFIXES`
entry in
+ /// `core/configs`, so the server's own unknown-variable check doesn't
cover this namespace
/// either).
pub const KNOWN_ENV_VARS: &'static [&'static str] = &[
"IGGY_KAFKA_IGGY_ADDR",
diff --git a/gateways/kafka/src/main.rs b/gateways/kafka/src/main.rs
index 140619531..7e01391fe 100644
--- a/gateways/kafka/src/main.rs
+++ b/gateways/kafka/src/main.rs
@@ -59,10 +59,10 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
}
/// The `IGGY_KAFKA_*` vars `load_config` itself reads. `IGGY_KAFKA_` is a
-/// `DELEGATED_ENV_VAR_PREFIXES` entry in `core/configs` (see that file's
comment), which trades
-/// away the central provider's typo-detection for this whole namespace - a
misspelled key here
-/// would otherwise silently no-op instead of surfacing anywhere.
`reject_unknown_kafka_env_vars`
-/// is this crate's own replacement for that lost check.
+/// `SERVER_ALLOWED_ENV_PREFIXES` entry in `core/configs`, which trades away
the server's
+/// unknown-variable check for this whole namespace - a misspelled key here
would otherwise
+/// silently no-op instead of surfacing anywhere.
`reject_unknown_kafka_env_vars` is this crate's
+/// own replacement for that lost check.
///
/// Deliberately excludes `IggyBridgeConfig::KNOWN_ENV_VARS`:
`reject_unknown_kafka_env_vars`
/// checks both lists rather than one merged copy, so a bridge var rename
can't silently desync
@@ -82,7 +82,7 @@ const KNOWN_KAFKA_ENV_VARS: &[&str] = &[
/// Rejects any `IGGY_KAFKA_*` env var not in [`KNOWN_KAFKA_ENV_VARS`] or
/// [`IggyBridgeConfig::KNOWN_ENV_VARS`] - a typo (e.g. `IGGY_KAFKA_BIN_ADDR`)
would otherwise be
-/// silently ignored: `core/configs`' central provider skips the whole
`IGGY_KAFKA_` prefix, and
+/// silently ignored: the server's check in `core/configs` skips the whole
`IGGY_KAFKA_` prefix, and
/// this crate's own `env_var()` only ever looks up exact known names, so
nothing reads the
/// misspelled var and nothing warns either. Checking both lists is
deliberate: a user who
/// exports a bridge var while `IGGY_KAFKA_BRIDGE_ENABLED` is off must not see
a spurious
