Refactor configuration module to use confique for environment variable management (#267)

danyi1212 · web-flow · commit be3872093d3e · 2025-06-05T14:25:51.000+03:00
* Refactor configuration module to use confique for environment variable management

* Fixed health check tests

- Updated the health check test setup to use the provided configuration directly instead of cloning it.
- Added a new test for health check success timing to validate the behavior with an increased timeout.
- Enhanced logging setup for better visibility during test execution.
diff --git a/pdp-server/Cargo.toml b/pdp-server/Cargo.toml
@@ -11,7 +11,7 @@ watchdog = { path = "../watchdog" }
 async-trait = "0.1.88"
 axum = "0.8.3"
 chrono = "0.4.40"
-config = "0.15.11"
+confique = { version = "0.3.0", features = ["toml"] }
 env_logger = "0.11.8"
 http = "1.3.1"
 log = "0.4.27"
diff --git a/pdp-server/src/api/health/handlers.rs b/pdp-server/src/api/health/handlers.rs
@@ -384,7 +384,7 @@ mod test {
 
         let mut config = crate::config::PDPConfig::for_test_with_mocks(&horizon_mock, &opa_mock);
         config.healthcheck_timeout = 0.5;
-        let fixture = TestFixture::with_config(config.clone(), opa_mock, horizon_mock).await;
+        let fixture = TestFixture::with_config(config, opa_mock, horizon_mock).await;
 
         let start_time = Instant::now();
         let response_with_cache = fixture.get("/health?check_cache=true").await;
@@ -428,9 +428,16 @@ mod test {
             "Concurrent check took too long: {:?}",
             duration_with_cache
         );
+    }
 
-        config.healthcheck_timeout = 2.0;
+    #[tokio::test]
+    async fn test_health_check_success_timing() {
+        TestFixture::setup_logger(LevelFilter::Info);
         let (horizon_mock_success, opa_mock_success) = setup_healthy_mocks().await;
+
+        let mut config =
+            crate::config::PDPConfig::for_test_with_mocks(&horizon_mock_success, &opa_mock_success);
+        config.healthcheck_timeout = 2.0;
         let fixture_success =
             TestFixture::with_config(config, opa_mock_success, horizon_mock_success).await;
 
diff --git a/pdp-server/src/config/cache.rs b/pdp-server/src/config/cache.rs
@@ -1,3 +1,4 @@
+use confique::Config;
 use serde::Deserialize;
 
 /// Specifies which cache store implementation to use
@@ -12,115 +13,37 @@ pub enum CacheStore {
 }
 
 /// Configuration for the caching subsystem
-#[derive(Debug, Deserialize, Clone)]
+#[derive(Debug, Config, Clone, Default)]
 pub struct CacheConfig {
     /// Cache TTL in seconds (default: 1 hour)
-    #[serde(default)]
+    #[config(env = "PDP_CACHE_TTL", default = 3600)]
     pub ttl: u32,
 
     /// Cache store type: "in-memory", "redis", or null (default)
-    #[serde(default)]
+    #[config(env = "PDP_CACHE_STORE", default = "none")]
     pub store: CacheStore,
 
     /// In-memory cache specific configuration
-    #[serde(default)]
+    #[config(nested)]
     pub memory: InMemoryConfig,
 
     /// Redis cache specific configuration
-    #[serde(default)]
+    #[config(nested)]
     pub redis: RedisConfig,
 }
 
-impl Default for CacheConfig {
-    fn default() -> Self {
-        Self {
-            ttl: 3600, // 1 hour
-            store: CacheStore::None,
-            memory: InMemoryConfig::default(),
-            redis: RedisConfig::default(),
-        }
-    }
-}
-
-impl CacheConfig {
-    /// Creates a new configuration from environment variables
-    pub fn from_env(config: &Self) -> Self {
-        // Start with the provided configuration
-        let mut result = config.clone();
-
-        // Apply TTL environment variable
-        if let Ok(ttl) = std::env::var("PDP_CACHE_TTL") {
-            if let Ok(parsed) = ttl.parse::<u32>() {
-                result.ttl = parsed;
-            }
-        }
-
-        // Apply store type from environment
-        result.store = match std::env::var("PDP_CACHE_STORE").as_deref() {
-            Ok("in-memory") => CacheStore::InMemory,
-            Ok("redis") => CacheStore::Redis,
-            Ok("none") => CacheStore::None,
-            _ => result.store.clone(), // Keep existing value if not specified
-        };
-
-        // Apply sub-configurations from environment
-        result.memory = InMemoryConfig::from_env(&result.memory);
-        result.redis = RedisConfig::from_env(&result.redis);
-
-        result
-    }
-}
-
 /// In-memory cache configuration options
-#[derive(Debug, Deserialize, Clone)]
+#[derive(Debug, Config, Clone, Default)]
 pub struct InMemoryConfig {
     /// Maximum capacity in MiB (default: 128 MiB)
-    #[serde(default)]
+    #[config(env = "PDP_CACHE_MEMORY_CAPACITY", default = 128)]
     pub capacity: usize,
 }
 
-impl Default for InMemoryConfig {
-    fn default() -> Self {
-        Self {
-            capacity: 128, // 128 MiB
-        }
-    }
-}
-
-impl InMemoryConfig {
-    /// Creates a new configuration from environment variables
-    pub fn from_env(config: &Self) -> Self {
-        // Start with the provided configuration
-        let mut result = config.clone();
-
-        if let Ok(capacity) = std::env::var("PDP_CACHE_MEMORY_CAPACITY") {
-            if let Ok(parsed) = capacity.parse::<usize>() {
-                result.capacity = parsed;
-            }
-        }
-
-        result
-    }
-}
-
 /// Redis cache configuration options
-#[derive(Debug, Deserialize, Clone, Default)]
+#[derive(Debug, Config, Clone, Default)]
 pub struct RedisConfig {
     /// Redis connection string
-    #[serde(default)]
+    #[config(env = "PDP_CACHE_REDIS_URL", default = "")]
     pub url: String,
 }
-
-impl RedisConfig {
-    /// Creates a new configuration from environment variables
-    pub fn from_env(config: &Self) -> Self {
-        // Start with the provided configuration
-        let mut result = config.clone();
-
-        if let Ok(url) = std::env::var("PDP_CACHE_REDIS_URL") {
-            result.url = url;
-        }
-
-        result
-    }
-}
diff --git a/pdp-server/src/config/horizon.rs b/pdp-server/src/config/horizon.rs
@@ -1,66 +1,49 @@
-use serde::Deserialize;
+use confique::Config;
 
 /// Configuration for the Horizon service
-#[derive(Debug, Deserialize, Clone)]
+#[derive(Debug, Config, Clone, Default)]
 pub struct HorizonConfig {
     /// Horizon service hostname (default: 0.0.0.0)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_HOST", default = "0.0.0.0")]
     pub host: String,
 
     /// Horizon service port (default: 7001)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_PORT", default = 7001)]
     pub port: u16,
 
     /// The timeout for Horizon client queries in seconds (default: 60)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_CLIENT_TIMEOUT", default = 60)]
     pub client_timeout: u64,
 
     /// Python interpreter path for running Horizon (default: python3)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_PYTHON_PATH", default = "python3")]
     pub python_path: String,
 
     /// Health check endpoint timeout in seconds (default: 1)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_HEALTH_CHECK_TIMEOUT", default = 1)]
     pub health_check_timeout: u64,
 
     /// Interval between health checks in seconds (default: 5)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_HEALTH_CHECK_INTERVAL", default = 5)]
     pub health_check_interval: u64,
 
     /// Number of consecutive health check failures before restarting (default: 12)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_HEALTH_CHECK_FAILURE_THRESHOLD", default = 12)]
     pub health_check_failure_threshold: u32,
 
     /// Initial delay before starting health checks in seconds (default: 5)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_STARTUP_DELAY", default = 5)]
     pub startup_delay: u64,
 
     /// Interval between service restart attempts in seconds (default: 1)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_RESTART_INTERVAL", default = 1)]
     pub restart_interval: u64,
 
     /// Service termination timeout in seconds (default: 30)
-    #[serde(default)]
+    #[config(env = "PDP_HORIZON_TERMINATION_TIMEOUT", default = 30)]
     pub termination_timeout: u64,
 }
 
-impl Default for HorizonConfig {
-    fn default() -> Self {
-        Self {
-            host: "0.0.0.0".to_string(),
-            port: 7001,
-            client_timeout: 60,
-            python_path: "python3".to_string(),
-            health_check_timeout: 1,
-            health_check_interval: 5,
-            health_check_failure_threshold: 12,
-            startup_delay: 5,
-            restart_interval: 1,
-            termination_timeout: 30,
-        }
-    }
-}
-
 impl HorizonConfig {
     /// Returns a properly formatted URL to the Horizon service with the given path
     pub fn get_url<S: Into<String>>(&self, path: S) -> String {
@@ -71,69 +54,4 @@ impl HorizonConfig {
             format!("http://{}:{}/{}", self.host, self.port, path)
         }
     }
-
-    /// Creates a new configuration from environment variables
-    pub fn from_env(config: &Self) -> Self {
-        // Start with the provided configuration
-        let mut result = config.clone();
-
-        // Apply environment variable overrides for horizon configuration
-        if let Ok(host) = std::env::var("PDP_HORIZON_HOST") {
-            result.host = host;
-        }
-
-        if let Ok(port) = std::env::var("PDP_HORIZON_PORT") {
-            if let Ok(parsed) = port.parse::<u16>() {
-                result.port = parsed;
-            }
-        }
-
-        if let Ok(python_path) = std::env::var("PDP_HORIZON_PYTHON_PATH") {
-            result.python_path = python_path;
-        }
-
-        if let Ok(timeout) = std::env::var("PDP_HORIZON_CLIENT_TIMEOUT") {
-            if let Ok(parsed) = timeout.parse::<u64>() {
-                result.client_timeout = parsed;
-            }
-        }
-
-        if let Ok(timeout) = std::env::var("PDP_HORIZON_HEALTH_CHECK_TIMEOUT") {
-            if let Ok(parsed) = timeout.parse::<u64>() {
-                result.health_check_timeout = parsed;
-            }
-        }
-
-        if let Ok(interval) = std::env::var("PDP_HORIZON_HEALTH_CHECK_INTERVAL") {
-            if let Ok(parsed) = interval.parse::<u64>() {
-                result.health_check_interval = parsed;
-            }
-        }
-
-        if let Ok(threshold) = std::env::var("PDP_HORIZON_HEALTH_CHECK_FAILURE_THRESHOLD") {
-            if let Ok(parsed) = threshold.parse::<u32>() {
-                result.health_check_failure_threshold = parsed;
-            }
-        }
-
-        if let Ok(delay) = std::env::var("PDP_HORIZON_STARTUP_DELAY") {
-            if let Ok(parsed) = delay.parse::<u64>() {
-                result.startup_delay = parsed;
-            }
-        }
-
-        if let Ok(interval) = std::env::var("PDP_HORIZON_RESTART_INTERVAL") {
-            if let Ok(parsed) = interval.parse::<u64>() {
-                result.restart_interval = parsed;
-            }
-        }
-
-        if let Ok(timeout) = std::env::var("PDP_HORIZON_TERMINATION_TIMEOUT") {
-            if let Ok(parsed) = timeout.parse::<u64>() {
-                result.termination_timeout = parsed;
-            }
-        }
-
-        result
-    }
 }
diff --git a/pdp-server/src/config/mod.rs b/pdp-server/src/config/mod.rs
diff --git a/pdp-server/src/config/opa.rs b/pdp-server/src/config/opa.rs
