feat(config): add FactConfigBuilder for better semantics

The new added FactConfigBuilder makes it obvious that the way of
building a configuration is by parsing a set of files and putting
together a FactConfig object from them and the CLI arguments. This
improves semantics and makes the code more streamlined.

Author: Molter73

fact/src/bpf.rs

@@ -217,12 +217,7 @@ mod bpf_tests {
     use tempfile::NamedTempFile;
     use tokio::{sync::watch, time::timeout};
 
-    use crate::{
-        config::{reloader::Reloader, FactConfig},
-        event::process::Process,
-        host_info,
-        metrics::exporter::Exporter,
-    };
+    use crate::{config, event::process::Process, host_info, metrics::exporter::Exporter};
 
     use super::*;
 
@@ -247,12 +242,10 @@ mod bpf_tests {
         let monitored_path = env!("CARGO_MANIFEST_DIR");
         let monitored_path = PathBuf::from(monitored_path);
         let paths = vec![monitored_path.clone()];
-        let mut config = FactConfig::default();
-        config.set_paths(paths);
-        let reloader = Reloader::from(config);
+        let (_, paths) = watch::channel(paths);
         executor.block_on(async {
-            let mut bpf = Bpf::new(reloader.paths(), reloader.config().ringbuf_size())
-                .expect("Failed to load BPF code");
+            let mut bpf =
+                Bpf::new(paths, config::DEFAULT_RINGBUFFER_SIZE).expect("Failed to load BPF code");
             let mut rx = bpf.subscribe();
             let (run_tx, run_rx) = watch::channel(true);
             // Create a metrics exporter, but don't start it

fact/src/config/builder.rs

@@ -0,0 +1,59 @@
+use std::{fs::read_to_string, path::PathBuf, sync::LazyLock};
+
+use anyhow::Context;
+use clap::Parser;
+
+use crate::config::FactCli;
+
+use super::FactConfig;
+
+pub(super) struct FactConfigBuilder {
+    files: Vec<PathBuf>,
+}
+
+impl FactConfigBuilder {
+    pub(super) fn new() -> Self {
+        let files = Vec::new();
+        FactConfigBuilder { files }
+    }
+
+    pub(super) fn add_files(
+        mut self,
+        files: &[impl Into<PathBuf> + AsRef<std::ffi::OsStr>],
+    ) -> Self {
+        for file in files {
+            self.files.push(file.into());
+        }
+        self
+    }
+
+    pub(super) fn files(&self) -> &[PathBuf] {
+        self.files.as_slice()
+    }
+
+    pub(super) fn build(&self) -> anyhow::Result<FactConfig> {
+        let mut config = self
+            .files
+            .iter()
+            .filter(|p| p.exists())
+            .map(|p| {
+                let content =
+                    read_to_string(p).with_context(|| format!("Failed to read {}", p.display()))?;
+                FactConfig::try_from(content.as_str())
+                    .with_context(|| format!("parsing error while processing {}", p.display()))
+            })
+            .try_fold(
+                FactConfig::default(),
+                |mut config: FactConfig, other: anyhow::Result<FactConfig>| {
+                    config.update(&other?);
+                    Ok::<FactConfig, anyhow::Error>(config)
+                },
+            )?;
+
+        // Once file configuration is handled, apply CLI arguments
+        static CLI_ARGS: LazyLock<FactConfig> = LazyLock::new(|| FactCli::parse().to_config());
+        config.update(&CLI_ARGS);
+
+        Ok(config)
+    }
+}

fact/src/config/mod.rs

@@ -1,26 +1,19 @@
 use std::{
-    fs::read_to_string,
     net::SocketAddr,
     path::{Path, PathBuf},
     str::FromStr,
-    sync::LazyLock,
 };
 
-use anyhow::{bail, Context};
+use anyhow::bail;
 use clap::Parser;
-use log::info;
 use yaml_rust2::{yaml, Yaml, YamlLoader};
 
+mod builder;
 pub mod reloader;
 #[cfg(test)]
 mod tests;
 
-const CONFIG_FILES: [&str; 4] = [
-    "/etc/stackrox/fact.yml",
-    "/etc/stackrox/fact.yaml",
-    "fact.yml",
-    "fact.yaml",
-];
+pub const DEFAULT_RINGBUFFER_SIZE: u32 = 8192;
 
 #[derive(Debug, Default, PartialEq, Eq, Clone)]
 pub struct FactConfig {
@@ -34,44 +27,6 @@ pub struct FactConfig {
 }
 
 impl FactConfig {
-    pub fn new() -> anyhow::Result<Self> {
-        let config = FactConfig::build()?;
-        info!("{config:#?}");
-        Ok(config)
-    }
-
-    fn build() -> anyhow::Result<FactConfig> {
-        let mut config = CONFIG_FILES
-            .iter()
-            .filter_map(|p| {
-                let p = Path::new(p);
-                if p.exists() {
-                    Some(p)
-                } else {
-                    None
-                }
-            })
-            .map(|p| {
-                let content =
-                    read_to_string(p).with_context(|| format!("Failed to read {}", p.display()))?;
-                FactConfig::try_from(content.as_str())
-                    .with_context(|| format!("parsing error while processing {}", p.display()))
-            })
-            .try_fold(
-                FactConfig::default(),
-                |mut config: FactConfig, other: anyhow::Result<FactConfig>| {
-                    config.update(&other?);
-                    Ok::<FactConfig, anyhow::Error>(config)
-                },
-            )?;
-
-        // Once file configuration is handled, apply CLI arguments
-        static CLI_ARGS: LazyLock<FactConfig> = LazyLock::new(|| FactCli::parse().to_config());
-        config.update(&CLI_ARGS);
-
-        Ok(config)
-    }
-
     pub fn update(&mut self, from: &FactConfig) {
         if let Some(paths) = from.paths.as_deref() {
             self.paths = Some(paths.to_owned());
@@ -110,7 +65,7 @@ impl FactConfig {
     }
 
     pub fn ringbuf_size(&self) -> u32 {
-        self.ringbuf_size.unwrap_or(8192)
+        self.ringbuf_size.unwrap_or(DEFAULT_RINGBUFFER_SIZE)
     }
 
     pub fn hotreload(&self) -> bool {

fact/src/config/reloader.rs

@@ -9,18 +9,66 @@ use tokio::{
     time::interval,
 };
 
-use super::{EndpointConfig, FactConfig, GrpcConfig, CONFIG_FILES};
+use super::{builder::FactConfigBuilder, EndpointConfig, FactConfig, GrpcConfig};
+
+const CONFIG_FILES: [&str; 4] = [
+    "/etc/stackrox/fact.yml",
+    "/etc/stackrox/fact.yaml",
+    "fact.yml",
+    "fact.yaml",
+];
 
 pub struct Reloader {
     config: FactConfig,
+    builder: FactConfigBuilder,
     endpoint: watch::Sender<EndpointConfig>,
     grpc: watch::Sender<GrpcConfig>,
     paths: watch::Sender<Vec<PathBuf>>,
-    files: HashMap<&'static str, i64>,
+    files: HashMap<PathBuf, i64>,
     trigger: Arc<Notify>,
 }
 
 impl Reloader {
+    pub fn new() -> anyhow::Result<Self> {
+        let builder = FactConfigBuilder::new().add_files(CONFIG_FILES.as_slice());
+        let config = builder.build()?;
+        info!("Startup configuration: {config:#?}");
+
+        let (endpoint, _) = watch::channel(config.endpoint.clone());
+        let (grpc, _) = watch::channel(config.grpc.clone());
+        let (paths, _) = watch::channel(config.paths().to_vec());
+        let trigger = Arc::new(Notify::new());
+        let files = builder
+            .files()
+            .iter()
+            .filter_map(|path| {
+                if path.exists() {
+                    let mtime = match path.metadata() {
+                        Ok(m) => m.mtime(),
+                        Err(e) => {
+                            warn!("Failed to stat {}: {e}", path.display());
+                            warn!("Configuration reloading may not work");
+                            return None;
+                        }
+                    };
+                    Some((path.clone(), mtime))
+                } else {
+                    None
+                }
+            })
+            .collect();
+
+        Ok(Reloader {
+            config,
+            builder,
+            endpoint,
+            grpc,
+            paths,
+            files,
+            trigger,
+        })
+    }
+
     /// Consume the reloader into a task
     ///
     /// The resulting task will handle reloading the configuration and
@@ -91,34 +139,33 @@ impl Reloader {
     fn update_cache(&mut self) -> bool {
         let mut res = false;
 
-        for file in CONFIG_FILES {
-            let path = PathBuf::from(file);
-            if path.exists() {
-                let mtime = match path.metadata() {
+        for file in self.builder.files() {
+            if file.exists() {
+                let mtime = match file.metadata() {
                     Ok(m) => m.mtime(),
                     Err(e) => {
-                        warn!("Failed to stat {file}: {e}");
+                        warn!("Failed to stat {}: {e}", file.display());
                         warn!("Configuration reloading may not work");
                         continue;
                     }
                 };
-                match self.files.get_mut(&file) {
+                match self.files.get_mut(file) {
                     Some(old) if *old == mtime => {}
                     Some(old) => {
-                        debug!("Updating '{file}'");
+                        debug!("Updating '{}'", file.display());
                         res = true;
                         *old = mtime;
                     }
                     None => {
-                        debug!("New configuration file '{file}'");
+                        debug!("New configuration file '{}'", file.display());
                         res = true;
-                        self.files.insert(file, mtime);
+                        self.files.insert(file.clone(), mtime);
                     }
                 }
-            } else if self.files.contains_key(&file) {
-                debug!("'{file}' no longer exists, removing from cache");
+            } else if self.files.contains_key(file) {
+                debug!("'{}' no longer exists, removing from cache", file.display());
                 res = true;
-                self.files.remove(&file);
+                self.files.remove(file);
             }
         }
         res
@@ -131,7 +178,7 @@ impl Reloader {
             return;
         }
 
-        let new = match FactConfig::build() {
+        let new = match self.builder.build() {
             Ok(config) => config,
             Err(e) => {
                 warn!("Configuration reloading failed: {e}");
@@ -178,40 +225,3 @@ impl Reloader {
         self.config = new;
     }
 }
-
-impl From<FactConfig> for Reloader {
-    fn from(config: FactConfig) -> Self {
-        let files = CONFIG_FILES
-            .iter()
-            .filter_map(|path| {
-                let p = PathBuf::from(path);
-                if p.exists() {
-                    let mtime = match p.metadata() {
-                        Ok(m) => m.mtime(),
-                        Err(e) => {
-                            warn!("Failed to stat {path}: {e}");
-                            warn!("Configuration reloading may not work");
-                            return None;
-                        }
-                    };
-                    Some((*path, mtime))
-                } else {
-                    None
-                }
-            })
-            .collect();
-        let (endpoint, _) = watch::channel(config.endpoint.clone());
-        let (grpc, _) = watch::channel(config.grpc.clone());
-        let (paths, _) = watch::channel(config.paths().to_vec());
-        let trigger = Arc::new(Notify::new());
-
-        Reloader {
-            config,
-            endpoint,
-            grpc,
-            paths,
-            files,
-            trigger,
-        }
-    }
-}

fact/src/config/tests.rs

@@ -831,6 +831,6 @@ fn defaults() {
     assert!(!config.endpoint.health_check());
     assert!(!config.skip_pre_flight());
     assert!(!config.json());
-    assert_eq!(config.ringbuf_size(), 8192);
+    assert_eq!(config.ringbuf_size(), DEFAULT_RINGBUFFER_SIZE);
     assert!(config.hotreload());
 }

fact/src/lib.rs

@@ -19,7 +19,7 @@ mod metrics;
 mod output;
 mod pre_flight;
 
-use config::FactConfig;
+use config::reloader::Reloader;
 use pre_flight::pre_flight;
 
 pub fn init_log() -> anyhow::Result<()> {
@@ -60,22 +60,22 @@ pub fn log_system_information() {
     info!("Hostname: {}", get_hostname());
 }
 
-pub async fn run(config: FactConfig) -> anyhow::Result<()> {
+pub async fn run() -> anyhow::Result<()> {
+    let reloader = Reloader::new()?;
+    let config_trigger = reloader.get_trigger();
+
     // Log system information as early as possible so we have it
     // available in case of a crash
     log_system_information();
     let (running, _) = watch::channel(true);
 
-    if !config.skip_pre_flight() {
+    if !reloader.config().skip_pre_flight() {
         debug!("Performing pre-flight checks");
         pre_flight().context("Pre-flight checks failed")?;
     } else {
         debug!("Skipping pre-flight checks");
     }
 
-    let reloader = config::reloader::Reloader::from(config);
-    let config_trigger = reloader.get_trigger();
-
     let mut bpf = Bpf::new(reloader.paths(), reloader.config().ringbuf_size())?;
     let exporter = Exporter::new(bpf.take_metrics()?);
 

fact/src/main.rs

@@ -1,9 +1,6 @@
-use fact::config::FactConfig;
-
 #[tokio::main]
 async fn main() -> anyhow::Result<()> {
     fact::init_log()?;
-    let config = FactConfig::new()?;
 
-    fact::run(config).await
+    fact::run().await
 }