-
Notifications
You must be signed in to change notification settings - Fork 2
Closed
Labels
enhancementNew feature or requestNew feature or request
Description
Summary
Currently, configuration is scattered throughout main() as individual variables parsed from environment variables. Extract this into a dedicated Config struct with proper validation, error handling, and a builder pattern for better testability and maintainability.
Current State
Configuration parsing is inline in main():
let target_url = std::env::var("TARGET_URL").expect("TARGET_URL must be set");
let max_concurrent: u32 = std::env::var("MAX_CONCURRENT")
.unwrap_or_else(|_| "10".to_string())
.parse()
.expect("MAX_CONCURRENT must be a valid u32");
// ... 20+ more env vars parsed similarlyProblems:
- Panics on invalid configuration (poor UX)
- No centralized validation
- Difficult to test configuration parsing
- No documentation of required vs optional fields
- Can't easily serialize/deserialize config
Proposed Solution
Config Struct
// src/config.rs
use std::time::Duration;
use std::path::PathBuf;
use thiserror::Error;
#[derive(Debug, Clone)]
pub struct Config {
// Required
pub target_url: String,
// Request configuration
pub request_method: RequestMethod,
pub post_body: Option<String>,
pub custom_headers: Vec<(String, String)>,
// TLS configuration
pub tls_config: TlsConfig,
// Load configuration
pub load_model: LoadModel,
pub max_concurrent: u32,
pub test_duration: Duration,
// DNS override
pub resolve_target_addr: Option<String>,
}
#[derive(Debug, Clone)]
pub struct TlsConfig {
pub skip_verify: bool,
pub client_cert_path: Option<PathBuf>,
pub client_key_path: Option<PathBuf>,
}
#[derive(Debug, Clone)]
pub enum RequestMethod {
Get,
Post,
}
#[derive(Error, Debug)]
pub enum ConfigError {
#[error("Missing required environment variable: {0}")]
MissingEnvVar(String),
#[error("Invalid value for {var}: {message}")]
InvalidValue { var: String, message: String },
#[error("mTLS configuration incomplete: both CLIENT_CERT_PATH and CLIENT_KEY_PATH required")]
IncompleteMtls,
#[error("Load model '{model}' requires: {required}")]
MissingLoadModelParams { model: String, required: String },
#[error("Invalid duration format: {0}")]
InvalidDuration(String),
#[error("File not found: {0}")]
FileNotFound(PathBuf),
}Builder/Loader Pattern
impl Config {
pub fn from_env() -> Result<Self, ConfigError> {
let target_url = env_required("TARGET_URL")?;
let tls_config = TlsConfig::from_env()?;
let load_model = LoadModel::from_env()?;
let config = Config {
target_url,
request_method: parse_request_method()?,
post_body: std::env::var("POST_BODY").ok(),
custom_headers: parse_headers()?,
tls_config,
load_model,
max_concurrent: env_parse_or("MAX_CONCURRENT", 10)?,
test_duration: parse_duration_from_env("TEST_DURATION", "1m")?,
resolve_target_addr: std::env::var("RESOLVE_TARGET_ADDR").ok(),
};
config.validate()?;
Ok(config)
}
fn validate(&self) -> Result<(), ConfigError> {
// Validate URL format
if !self.target_url.starts_with("http://") && !self.target_url.starts_with("https://") {
return Err(ConfigError::InvalidValue {
var: "TARGET_URL".into(),
message: "Must start with http:// or https://".into(),
});
}
// Validate max_concurrent
if self.max_concurrent == 0 {
return Err(ConfigError::InvalidValue {
var: "MAX_CONCURRENT".into(),
message: "Must be greater than 0".into(),
});
}
// Validate mTLS (both or neither)
if self.tls_config.client_cert_path.is_some() != self.tls_config.client_key_path.is_some() {
return Err(ConfigError::IncompleteMtls);
}
// Validate load model params
self.load_model.validate()?;
Ok(())
}
}Helper Functions
fn env_required(name: &str) -> Result<String, ConfigError> {
std::env::var(name).map_err(|_| ConfigError::MissingEnvVar(name.into()))
}
fn env_parse_or<T: std::str::FromStr>(name: &str, default: T) -> Result<T, ConfigError>
where
T::Err: std::fmt::Display,
{
match std::env::var(name) {
Ok(val) => val.parse().map_err(|e| ConfigError::InvalidValue {
var: name.into(),
message: e.to_string(),
}),
Err(_) => Ok(default),
}
}Usage in main()
#[tokio::main]
async fn main() {
let config = match Config::from_env() {
Ok(c) => c,
Err(e) => {
eprintln!("Configuration error: {}", e);
eprintln!("\nRequired environment variables:");
eprintln!(" TARGET_URL - The URL to load test");
eprintln!("\nOptional environment variables:");
eprintln!(" MAX_CONCURRENT - Maximum concurrent requests (default: 10)");
// ... more help text
std::process::exit(1);
}
};
println!("Configuration loaded successfully:");
println!(" Target URL: {}", config.target_url);
println!(" Load Model: {:?}", config.load_model);
// ...
}Acceptance Criteria
-
Configstruct contains all configuration fields -
Config::from_env()returnsResult<Config, ConfigError> - All configuration errors have descriptive messages
- Invalid configurations print help text and exit cleanly (no panics)
- mTLS validation ensures both cert and key are provided
- Load model validation ensures required params are present
- Config struct is testable without environment variables
- Documentation comments on all public fields
Testing Support
The struct should support testing without env vars:
impl Config {
#[cfg(test)]
pub fn for_testing() -> Self {
Config {
target_url: "https://example.com".into(),
request_method: RequestMethod::Get,
post_body: None,
custom_headers: vec![],
tls_config: TlsConfig::default(),
load_model: LoadModel::Concurrent,
max_concurrent: 10,
test_duration: Duration::from_secs(60),
resolve_target_addr: None,
}
}
}Benefits
- Better UX: Descriptive error messages instead of panics
- Testability: Config struct can be constructed in tests without env vars
- Validation: Centralized validation catches configuration errors early
- Documentation: Struct fields serve as configuration documentation
- Maintainability: Single place to add/modify configuration options
Priority
Medium - Improves developer and user experience significantly, pairs well with #15 (modular refactor).
Related Issues
- Refactor: Split main.rs into modular structure #15 - Refactor main.rs into modules (config.rs is part of that)
- Add unit tests for configuration parsing and validation #18 - Add unit tests for configuration parsing (tests this struct)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
enhancementNew feature or requestNew feature or request