Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danyi1212 merged 2 commits into from
Jun 5, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9de20d
Refactor configuration module to use confique for environment variabl…
danyi1212 Jun 3, 2025
73b5002
Fixed health check tests
danyi1212 Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pdp-server/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
11 changes: 9 additions & 2 deletions pdp-server/src/api/health/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
97 changes: 10 additions & 87 deletions pdp-server/src/config/cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use confique::Config;
use serde::Deserialize;

/// Specifies which cache store implementation to use
Expand All @@ -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)]
Copy link
Contributor

@omer9564 omer9564 Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there isn't a prefix implementation or auto env generation here ?

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,
Copy link
Contributor

@omer9564 omer9564 Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you used to have default of ::default() here, what will happen here ?

Copy link
Contributor

@omer9564 omer9564 Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you used to have default of ::default() here, what will happen here ?


/// 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
}
}
106 changes: 12 additions & 94 deletions pdp-server/src/config/horizon.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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
}
}
Loading
Loading