From e3d51c49dd4412c528f78a541bbab5d0f596a41c Mon Sep 17 00:00:00 2001 From: Leonardo Forchini Date: Wed, 21 Oct 2026 09:25:08 +0100 Subject: [PATCH] Refactor config loading mechanism Instead of having three separate implementations of 'try to load, use default if ENOENT', introduce a canonical config loading function. Signed-off-by: Leonardo Forchini --- src/backends/qemu/mod.rs | 8 ++------ src/config.rs | 35 +++++++++++++++++++++++------------ src/engines/threshold.rs | 9 ++------- src/main.rs | 21 ++++----------------- 4 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/backends/qemu/mod.rs b/src/backends/qemu/mod.rs index 51265dc0..13f3ba9a 100644 --- a/src/backends/qemu/mod.rs +++ b/src/backends/qemu/mod.rs @@ -31,7 +31,7 @@ use serde::{Deserialize, Serialize}; use crate::{ backends::{Backend, BackendClientError, BackendRegistration}, - config::{ConfigError, load_json_with_includes}, + config::{self, ConfigError}, instance::{Instance, InstanceClient}, util::Path, }; @@ -83,11 +83,7 @@ impl QemuConfig { /// built-in defaults. pub fn from_dir(dir: &Path) -> Result { let path = Path::new(&dir.join("qemu.json")); - if path.exists() { - load_json_with_includes(path) - } else { - Ok(Self::default()) - } + config::load_config_or_default(&path) } } diff --git a/src/config.rs b/src/config.rs index 8c7b106e..3eb38337 100644 --- a/src/config.rs +++ b/src/config.rs @@ -169,25 +169,36 @@ struct ConfigEnvelope { rest: serde_json::Value, } -/// Load any JSON document that opts into the `include` -/// mechanism. Engines call this on their own config file so -/// site-specific overrides can live in a sibling file. +/// Attempt to load a JSON config file. If not found, fall back on the Default +/// implementation. +pub fn load_config_or_default Deserialize<'de> + Default>( + path: impl AsRef, +) -> Result { + match load_json_with_includes(path) { + r @ Ok(_) => r, + Err(ConfigError::Io(e)) if e.kind() == std::io::ErrorKind::NotFound => { + Ok(Default::default()) + } + e @ Err(_) => e, + } +} + +/// Load any JSON document that opts into the `include` mechanism. Engines +/// call this on their own config file so site-specific overrides can live in a +/// sibling file. pub fn load_json_with_includes Deserialize<'de>>( - path: impl AsRef, + path: impl AsRef, ) -> Result { let path = path.as_ref(); let value = load_config_value(path)?; - let out: T = serde_json::from_value(value)?; - Ok(out) + Ok(serde_json::from_value(value)?) } -fn load_config_value(path: &Path) -> Result { +fn load_config_value(path: &std::path::Path) -> Result { let data = std::fs::read_to_string(path)?; let env: ConfigEnvelope = serde_json::from_str(&data)?; let mut merged = if let Some(rel) = env.include { - let path = Path::new("."); - let inc_path = Path::new(&path.parent().unwrap_or_else(|| &path).join(&rel)); - load_config_value(&inc_path)? + load_config_value(&path.parent().unwrap_or(path).join(&rel))? } else { serde_json::Value::Object(Default::default()) }; @@ -297,7 +308,7 @@ mod tests { ) .unwrap(); - let cfg: Config = load_json_with_includes(Path::new(path.to_str().unwrap())).unwrap(); + let cfg: Config = load_config_or_default(Path::new(path.to_str().unwrap())).unwrap(); assert_eq!(cfg.engine, "threshold"); assert!((cfg.scale_poll_secs - 7.5).abs() < f64::EPSILON); assert_eq!( @@ -324,7 +335,7 @@ mod tests { ) .unwrap(); - let err = load_json_with_includes::(Path::new(path.to_str().unwrap())).unwrap_err(); + let err = load_config_or_default::(Path::new(path.to_str().unwrap())).unwrap_err(); assert!(matches!(err, ConfigError::SerdeJson(_))); } diff --git a/src/engines/threshold.rs b/src/engines/threshold.rs index 65d358ae..dc5a51ad 100644 --- a/src/engines/threshold.rs +++ b/src/engines/threshold.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize}; use tokio::sync::Mutex; use crate::{ - config::{ConfigError, deserialize_percent, serialize_percent}, + config::{self, ConfigError, deserialize_percent, serialize_percent}, engines::{ AppliedOutcome, EngineError, EngineRegistration, EngineTickContext, ScaleAction, ScalingEngine, @@ -409,12 +409,7 @@ impl ThresholdEngine { /// Load `threshold.json`, falling back to built-in defaults when absent. pub fn from_config_dir(dir: &Path) -> Result { let path = Path::new(&dir.join(format!("{ENGINE_NAME}.json"))); - // TODO TOCTOU, blindly load and return default if ENOENT - let cfg: ThresholdConfig = if path.exists() { - crate::config::load_json_with_includes(path)? - } else { - ThresholdConfig::default() - }; + let cfg: ThresholdConfig = config::load_config_or_default(&path)?; cfg.validate()?; Ok(Self::new(cfg)) } diff --git a/src/main.rs b/src/main.rs index d276d99e..212d0084 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,18 +5,15 @@ //! `io-thread-controller` daemon entry point. -use std::io::ErrorKind; - use clap::{self, CommandFactory, FromArgMatches, Parser}; use io_thread_controller::{ - backends::BackendClientError, - backends::registered_backends, - config::{Config, ConfigError, dump_default_config, load_json_with_includes, validate_config}, + backends::{BackendClientError, registered_backends}, + config::{Config, ConfigError, dump_default_config, load_config_or_default, validate_config}, daemon::{DaemonError, VERSION, run}, util::Path, }; use thiserror::Error; -use tracing::{debug, info, instrument}; +use tracing::{debug, info}; use tracing_subscriber::fmt::format::FmtSpan; #[derive(Error, Debug)] @@ -130,7 +127,7 @@ async fn main() -> Result<(), IoThreadControllerError> { "io-thread-controller starting" ); - let mut cfg = load_daemon_config(&cli.config)?; + let mut cfg: Config = load_config_or_default(&cli.config)?; debug!("loaded configuration {}", &cli.config); if cli.print_status_header { cfg.print_status_header = true; @@ -144,16 +141,6 @@ async fn main() -> Result<(), IoThreadControllerError> { Ok(()) } -/// Load defaults only for an absent file; propagate every other open failure. -#[instrument] -fn load_daemon_config(path: &Path) -> Result { - match std::fs::File::open(path) { - Ok(_) => load_json_with_includes(path), - Err(error) if error.kind() == ErrorKind::NotFound => Ok(Config::default()), - Err(error) => Err(error)?, - } -} - /// Install the process-wide tracing subscriber. fn init_logging(filter: &str, style: LogStyle) { // Auto-detection: systemd sets `JOURNAL_STREAM=:`