From b9ed127c797b75cfc903fb6dfd8122fbc03d2cb2 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Mon, 1 Jun 2026 15:59:30 +0200 Subject: [PATCH] test: add unit tests for environment variable bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds coverage for all FACT_* env-var bindings in FactCli: - env_vars: each env var populates the correct FactConfig field - env_vars_override_yaml: env vars take precedence over YAML config - env_vars_invalid_values: invalid values produce ValueValidation errors Introduces EnvVar and EnvVarGuard helper types that together acquire ENV_MUTEX, set the variable, and guarantee removal on drop — even if the test panics. std::env::set_var is not safe in multi-threaded programs; the mutex serialises all env-var mutations within the suite as the least invasive available mitigation. Closes #730 Assisted-by: Claude Sonnet 4.6 --- fact/src/config/tests.rs | 377 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 377 insertions(+) diff --git a/fact/src/config/tests.rs b/fact/src/config/tests.rs index 0079d672..15a32164 100644 --- a/fact/src/config/tests.rs +++ b/fact/src/config/tests.rs @@ -1,3 +1,9 @@ +use std::{ + error::Error, + fmt::Display, + sync::{Mutex, MutexGuard}, +}; + use super::*; #[test] @@ -1100,3 +1106,374 @@ fn defaults() { assert_eq!(config.bpf.inodes_max(), 65536); assert!(config.hotreload()); } + +static ENV_MUTEX: Mutex<()> = Mutex::new(()); + +/// RAII guard that holds the `ENV_MUTEX` lock and removes the named environment +/// variable when dropped, ensuring both are released even if the test panics +/// after calling [`EnvVar::set`]. +/// +/// The mutex is released after the variable is removed, so no other test can +/// observe the env var in a partially-cleaned-up state. +struct EnvVarGuard { + name: &'static str, + _guard: MutexGuard<'static, ()>, +} + +impl Drop for EnvVarGuard { + fn drop(&mut self) { + unsafe { std::env::remove_var(self.name) }; + } +} + +/// An environment variable key-value pair used in tests to exercise env-var +/// bindings in [`FactCli`]. +/// +/// `std::env::set_var` is not safe to call from multi-threaded programs, as +/// other threads may be reading the environment concurrently. There is no +/// better alternative at the moment: the test binary is multi-threaded by +/// default, and the only truly sound options would be forcing +/// `RUST_TEST_THREADS=1` or switching to a single-threaded runtime, both of +/// which are more invasive than the mutex approach used here. [`EnvVar::set`] +/// acquires `ENV_MUTEX` to at least serialise all env-var mutations within our +/// own test suite. +#[derive(Clone, Copy)] +struct EnvVar { + name: &'static str, + value: &'static str, +} + +impl EnvVar { + /// Acquires `ENV_MUTEX`, sets the environment variable, and returns an + /// [`EnvVarGuard`] that holds the lock and removes the variable on drop. + fn set(self) -> EnvVarGuard { + let _guard = ENV_MUTEX.lock().unwrap(); + unsafe { std::env::set_var(self.name, self.value) }; + EnvVarGuard { + name: self.name, + _guard, + } + } +} + +impl Display for EnvVar { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}={}", self.name, self.value) + } +} + +fn with_env_var(env: EnvVar) -> Result { + let _guard = env.set(); + FactCli::try_parse_from(["fact"]).map(|cli| cli.to_config()) +} + +#[test] +fn env_vars() { + let tests = [ + ( + EnvVar { + name: "FACT_INODES_MAX", + value: "1024", + }, + FactConfig { + bpf: BpfConfig { + inodes_max: Some(1024), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_RINGBUF_SIZE", + value: "128", + }, + FactConfig { + bpf: BpfConfig { + ringbuf_size: Some(128), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_PATHS", + value: "/etc:/var/log", + }, + FactConfig { + paths: Some(vec![PathBuf::from("/etc"), PathBuf::from("/var/log")]), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_JSON", + value: "true", + }, + FactConfig { + json: Some(true), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_SCAN_INTERVAL", + value: "45.5", + }, + FactConfig { + scan_interval: Some(Duration::from_secs_f64(45.5)), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_RATE_LIMIT", + value: "500", + }, + FactConfig { + rate_limit: Some(500), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_URL", + value: "https://svc.sensor.stackrox:9090", + }, + FactConfig { + grpc: GrpcConfig { + url: Some(String::from("https://svc.sensor.stackrox:9090")), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_CERTS", + value: "/etc/stackrox/certs", + }, + FactConfig { + grpc: GrpcConfig { + certs: Some(PathBuf::from("/etc/stackrox/certs")), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_ENDPOINT_ADDRESS", + value: "0.0.0.0:8080", + }, + FactConfig { + endpoint: EndpointConfig { + address: Some(SocketAddr::from(([0, 0, 0, 0], 8080))), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_ENDPOINT_EXPOSE_METRICS", + value: "true", + }, + FactConfig { + endpoint: EndpointConfig { + expose_metrics: Some(true), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_ENDPOINT_HEALTH_CHECK", + value: "true", + }, + FactConfig { + endpoint: EndpointConfig { + health_check: Some(true), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_SKIP_PRE_FLIGHT", + value: "true", + }, + FactConfig { + skip_pre_flight: Some(true), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_HOTRELOAD", + value: "true", + }, + FactConfig { + hotreload: Some(true), + ..Default::default() + }, + ), + ]; + for (env, expected) in tests { + let config = with_env_var(env).expect("env-var CLI parse failed"); + assert_eq!(config, expected, "env var {env}"); + } +} + +#[test] +fn env_vars_override_yaml() { + let tests = [ + ( + EnvVar { + name: "FACT_INODES_MAX", + value: "2048", + }, + "bpf:\n inodes_max: 1024", + FactConfig { + bpf: BpfConfig { + inodes_max: Some(2048), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_RINGBUF_SIZE", + value: "256", + }, + "bpf:\n ringbuf_size: 128", + FactConfig { + bpf: BpfConfig { + ringbuf_size: Some(256), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_URL", + value: "https://override:9090", + }, + "grpc:\n url: 'https://original:9090'", + FactConfig { + grpc: GrpcConfig { + url: Some(String::from("https://override:9090")), + ..Default::default() + }, + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_PATHS", + value: "/var/log", + }, + "paths:\n- /etc", + FactConfig { + paths: Some(vec![PathBuf::from("/var/log")]), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_JSON", + value: "true", + }, + "json: false", + FactConfig { + json: Some(true), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_SCAN_INTERVAL", + value: "60", + }, + "scan_interval: 30", + FactConfig { + scan_interval: Some(Duration::from_secs(60)), + ..Default::default() + }, + ), + ( + EnvVar { + name: "FACT_RATE_LIMIT", + value: "1000", + }, + "rate_limit: 500", + FactConfig { + rate_limit: Some(1000), + ..Default::default() + }, + ), + ]; + for (env, yaml, expected) in tests { + let mut config = match FactConfig::try_from(yaml) { + Ok(c) => c, + Err(e) => panic!("Failed to parse YAML\n\tError: {e}\n\tyaml: {yaml}"), + }; + config.update(&with_env_var(env).expect("env-var CLI parse failed")); + assert_eq!(config, expected, "env var {env} should override yaml",); + } +} + +#[test] +fn env_vars_invalid_values() { + let tests = [ + ( + EnvVar { + name: "FACT_INODES_MAX", + value: "not_a_number", + }, + "invalid digit found in string", + ), + ( + EnvVar { + name: "FACT_RINGBUF_SIZE", + value: "not_a_number", + }, + "invalid digit found in string", + ), + ( + EnvVar { + name: "FACT_ENDPOINT_ADDRESS", + value: "not_an_address", + }, + "invalid socket address syntax", + ), + ( + EnvVar { + name: "FACT_SCAN_INTERVAL", + value: "not_a_float", + }, + "invalid float literal", + ), + ( + EnvVar { + name: "FACT_RATE_LIMIT", + value: "not_a_number", + }, + "invalid digit found in string", + ), + ]; + for (env, expected) in tests { + let Err(err) = with_env_var(env) else { + panic!("Expected Error was not caught - expected: {expected}"); + }; + assert_eq!( + err.source().map(|e| e.to_string()).unwrap_or_default(), + expected, + ); + } +}