From fb4c6e4bdc7ba408c1c4d3bf4ea18c57fd23ed50 Mon Sep 17 00:00:00 2001 From: Celestial Date: Fri, 13 Feb 2026 20:27:17 +0100 Subject: [PATCH] feat(architecture): implement phase 2 config decoupling --- src/config/apply.rs | 36 ++++++++++++---- src/config/apply/scenario.rs | 43 ++++++++++++++++--- src/config/apply/section_tail.rs | 5 ++- src/config/tests.rs | 36 ++++++++-------- .../controller/manual/run_lifecycle.rs | 16 ++++--- src/entry/plan/build.rs | 17 ++++---- src/fuzzing.rs | 22 ++++++---- src/wasm_runtime/loader.rs | 10 ++++- 8 files changed, 126 insertions(+), 59 deletions(-) diff --git a/src/config/apply.rs b/src/config/apply.rs index df76d6d..671b0b5 100644 --- a/src/config/apply.rs +++ b/src/config/apply.rs @@ -6,28 +6,46 @@ mod section_runtime; mod section_tail; mod util; +use std::collections::BTreeMap; + use clap::ArgMatches; use crate::args::TesterArgs; +use crate::config::types::ScenarioConfig; use crate::error::{AppError, AppResult, ConfigError}; use super::types::ConfigFile; +use scenario::ScenarioDefaults; -/// Applies configuration values to CLI arguments. +/// Builds config-driven overrides over preset arguments. /// /// # Errors /// /// Returns an error when config values are invalid or conflict with CLI options. pub fn apply_config( - args: &mut TesterArgs, + preset_args: TesterArgs, matches: &ArgMatches, - config: &ConfigFile, -) -> AppResult<()> { - validate_config_conflicts(config)?; - section_basic::apply_basic_config(args, matches, config)?; - section_runtime::apply_runtime_config(args, matches, config)?; - section_tail::apply_tail_config(args, matches, config)?; - Ok(()) + mut config: ConfigFile, +) -> AppResult<(TesterArgs, Option>)> { + validate_config_conflicts(&config)?; + + // Merge order is centralized here and intentionally explicit: + // command-line values in `preset_args` win, config fills missing values, + // and untouched fields keep preset defaults. + let mut effective_args = preset_args; + section_basic::apply_basic_config(&mut effective_args, matches, &config)?; + section_runtime::apply_runtime_config(&mut effective_args, matches, &config)?; + let scenario_defaults = ScenarioDefaults::new( + effective_args.url.clone(), + effective_args.method, + effective_args.data.clone(), + effective_args.headers.clone(), + ); + section_tail::apply_tail_config(&mut effective_args, matches, &config, &scenario_defaults)?; + + let scenario_registry = config.scenarios.take(); + + Ok((effective_args, scenario_registry)) } fn validate_config_conflicts(config: &ConfigFile) -> AppResult<()> { diff --git a/src/config/apply/scenario.rs b/src/config/apply/scenario.rs index fb91151..0a8f0d9 100644 --- a/src/config/apply/scenario.rs +++ b/src/config/apply/scenario.rs @@ -1,9 +1,37 @@ -use crate::args::{Scenario, ScenarioStep, TesterArgs, parse_header}; +use crate::args::{HttpMethod, Scenario, ScenarioStep, parse_header}; use crate::error::{AppError, AppResult, ConfigError}; use super::super::types::{SCENARIO_SCHEMA_VERSION, ScenarioConfig}; -pub(crate) fn parse_scenario(config: &ScenarioConfig, args: &TesterArgs) -> AppResult { +#[derive(Debug, Clone)] +pub(crate) struct ScenarioDefaults { + pub(crate) base_url: Option, + pub(crate) method: HttpMethod, + pub(crate) body: String, + pub(crate) headers: Vec<(String, String)>, +} + +impl ScenarioDefaults { + #[must_use] + pub(crate) const fn new( + base_url: Option, + method: HttpMethod, + body: String, + headers: Vec<(String, String)>, + ) -> Self { + Self { + base_url, + method, + body, + headers, + } + } +} + +pub(crate) fn parse_scenario( + config: &ScenarioConfig, + defaults: &ScenarioDefaults, +) -> AppResult { if let Some(schema_version) = config.schema_version && schema_version != SCENARIO_SCHEMA_VERSION { @@ -16,9 +44,12 @@ pub(crate) fn parse_scenario(config: &ScenarioConfig, args: &TesterArgs) -> AppR return Err(AppError::config(ConfigError::ScenarioMissingSteps)); } - let base_url = config.base_url.clone().or_else(|| args.url.clone()); - let default_method = config.method.unwrap_or(args.method); - let default_body = config.data.clone().unwrap_or_else(|| args.data.clone()); + let base_url = config + .base_url + .clone() + .or_else(|| defaults.base_url.clone()); + let default_method = config.method.unwrap_or(defaults.method); + let default_body = config.data.clone().unwrap_or_else(|| defaults.body.clone()); let default_headers = if let Some(headers) = config.headers.as_ref() { let mut parsed = Vec::with_capacity(headers.len()); @@ -30,7 +61,7 @@ pub(crate) fn parse_scenario(config: &ScenarioConfig, args: &TesterArgs) -> AppR } parsed } else { - args.headers.clone() + defaults.headers.clone() }; let vars = config.vars.clone().unwrap_or_default(); diff --git a/src/config/apply/section_tail.rs b/src/config/apply/section_tail.rs index 4e2aced..5cb219c 100644 --- a/src/config/apply/section_tail.rs +++ b/src/config/apply/section_tail.rs @@ -5,13 +5,14 @@ use crate::error::AppResult; use super::super::types::ConfigFile; use super::distributed::apply_distributed_config; -use super::scenario::parse_scenario; +use super::scenario::{ScenarioDefaults, parse_scenario}; use super::util::{ensure_positive_u64, ensure_positive_usize, is_cli}; pub(super) fn apply_tail_config( args: &mut TesterArgs, matches: &ArgMatches, config: &ConfigFile, + scenario_defaults: &ScenarioDefaults, ) -> AppResult<()> { if !is_cli(matches, "metrics_max") && let Some(max) = config.metrics_max @@ -56,7 +57,7 @@ pub(super) fn apply_tail_config( } if let Some(scenario) = config.scenario.as_ref() { - args.scenario = Some(parse_scenario(scenario, args)?); + args.scenario = Some(parse_scenario(scenario, scenario_defaults)?); } if let Some(sinks) = config.sinks.as_ref() { diff --git a/src/config/tests.rs b/src/config/tests.rs index 7272d9e..aa6e8bc 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -147,10 +147,10 @@ aws_sigv4 = "aws:amz:us-east-1:service" let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - apply_config(&mut args, &matches, &config)?; + let args = apply_config(args, &matches, config)?.0; if args.proxy_url.as_deref() != Some("http://127.0.0.1:8080") { return Err(AppError::config("Unexpected proxy_url")); @@ -264,10 +264,10 @@ fn apply_config_rejects_ipv4_ipv6_conflict() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - if apply_config(&mut args, &matches, &config).is_ok() { + if apply_config(args, &matches, config).is_ok() { return Err(AppError::config("Expected ipv4/ipv6 conflict error")); } @@ -284,10 +284,10 @@ fn apply_config_rejects_conflicting_body_sources() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - if apply_config(&mut args, &matches, &config).is_ok() { + if apply_config(args, &matches, config).is_ok() { return Err(AppError::config("Expected conflict error")); } @@ -304,10 +304,10 @@ fn apply_config_respects_cli_overrides() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest", "--url", "http://from-cli", "--no-charts"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - apply_config(&mut args, &matches, &config)?; + let args = apply_config(args, &matches, config)?.0; if args.url.as_deref() != Some("http://from-cli") { return Err(AppError::config("Expected CLI url to win")); @@ -337,10 +337,10 @@ fn apply_config_load_profile_rate_to_rpm() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - apply_config(&mut args, &matches, &config)?; + let args = apply_config(args, &matches, config)?.0; let load = match args.load_profile { Some(load) => load, @@ -380,10 +380,10 @@ fn apply_config_rejects_load_and_rate_conflict() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - let result = apply_config(&mut args, &matches, &config); + let result = apply_config(args, &matches, config); if result.is_err() { Ok(()) } else { @@ -425,10 +425,10 @@ fn apply_config_sets_warmup_and_tls() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - apply_config(&mut args, &matches, &config)?; + let args = apply_config(args, &matches, config)?.0; if args.warmup != Some(Duration::from_secs(10)) { return Err(AppError::config("Expected warmup to be 10s")); @@ -472,10 +472,10 @@ fn apply_config_parses_scenario() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - apply_config(&mut args, &matches, &config)?; + let args = apply_config(args, &matches, config)?.0; let scenario = match args.scenario { Some(scenario) => scenario, @@ -530,10 +530,10 @@ fn apply_config_sets_distributed_fields() -> AppResult<()> { let cmd = TesterArgs::command(); let matches = cmd.get_matches_from(["strest"]); - let mut args = TesterArgs::from_arg_matches(&matches) + let args = TesterArgs::from_arg_matches(&matches) .map_err(|err| AppError::config(format!("parse args failed: {}", err)))?; - apply_config(&mut args, &matches, &config)?; + let args = apply_config(args, &matches, config)?.0; if args.agent_join.as_deref() != Some("127.0.0.1:9009") { return Err(AppError::config("Unexpected agent_join")); diff --git a/src/distributed/controller/manual/run_lifecycle.rs b/src/distributed/controller/manual/run_lifecycle.rs index 7fe0e37..1c1d698 100644 --- a/src/distributed/controller/manual/run_lifecycle.rs +++ b/src/distributed/controller/manual/run_lifecycle.rs @@ -7,7 +7,7 @@ use tokio::sync::watch; use tokio::time::{Instant, MissedTickBehavior}; use crate::args::{Scenario, TesterArgs}; -use crate::config::apply::scenario::parse_scenario; +use crate::config::apply::scenario::{ScenarioDefaults, parse_scenario}; use crate::ui::{model::UiData, render::setup_render_ui}; use super::super::control::{ControlError, ControlStartRequest}; @@ -161,9 +161,15 @@ pub(super) fn resolve_scenario_for_run( request: &ControlStartRequest, scenario_state: &mut ScenarioState, ) -> Result, ControlError> { + let scenario_defaults = ScenarioDefaults::new( + args.url.clone(), + args.method, + args.data.clone(), + args.headers.clone(), + ); if let Some(config) = request.scenario.as_ref() { - let scenario = - parse_scenario(config, args).map_err(|err| ControlError::new(400, err.to_string()))?; + let scenario = parse_scenario(config, &scenario_defaults) + .map_err(|err| ControlError::new(400, err.to_string()))?; if let Some(name) = request.scenario_name.as_ref() { scenario_state.named.insert(name.clone(), config.clone()); } @@ -175,8 +181,8 @@ pub(super) fn resolve_scenario_for_run( .named .get(name) .ok_or_else(|| ControlError::new(404, "Scenario not found."))?; - let scenario = - parse_scenario(config, args).map_err(|err| ControlError::new(400, err.to_string()))?; + let scenario = parse_scenario(config, &scenario_defaults) + .map_err(|err| ControlError::new(400, err.to_string()))?; return Ok(Some(scenario)); } diff --git a/src/entry/plan/build.rs b/src/entry/plan/build.rs index 26f7b80..52b92d4 100644 --- a/src/entry/plan/build.rs +++ b/src/entry/plan/build.rs @@ -70,7 +70,7 @@ pub(crate) fn build_plan(mut args: TesterArgs, matches: &ArgMatches) -> AppResul return Ok(RunPlan::Replay(to_replay_run_command(args))); } - let scenario_registry = apply_config(&mut args, matches)?; + let (mut args, scenario_registry) = apply_config(args, matches)?; apply_output_aliases(&mut args)?; validate_db_logging(&args)?; @@ -131,16 +131,15 @@ pub(crate) fn build_plan(mut args: TesterArgs, matches: &ArgMatches) -> AppResul } fn apply_config( - args: &mut TesterArgs, + args: TesterArgs, matches: &ArgMatches, -) -> AppResult>> { - let mut scenario_registry = None; - let mut loaded_config = crate::config::load_config(args.config.as_deref())?; - if let Some(config) = loaded_config.as_mut() { - scenario_registry = config.scenarios.take(); - crate::config::apply_config(args, matches, config)?; +) -> AppResult<(TesterArgs, Option>)> { + let loaded_config = crate::config::load_config(args.config.as_deref())?; + if let Some(config) = loaded_config { + let overrides = crate::config::apply_config(args, matches, config)?; + return Ok(overrides); } - Ok(scenario_registry) + Ok((args, None)) } fn apply_output_aliases(args: &mut TesterArgs) -> AppResult<()> { diff --git a/src/fuzzing.rs b/src/fuzzing.rs index cb4ecea..d76d02a 100644 --- a/src/fuzzing.rs +++ b/src/fuzzing.rs @@ -8,7 +8,7 @@ use crate::args::{ PositiveU64, PositiveUsize, Scenario, ScenarioStep, TesterArgs, TlsVersion, parse_header, parsers::parse_duration_arg, }; -use crate::config::apply::scenario::parse_scenario; +use crate::config::apply::scenario::{ScenarioDefaults, parse_scenario}; use crate::config::types::{ConfigFile, LoadConfig, ScenarioConfig}; use crate::config::{apply_config, parse_duration_value}; use crate::error::{AppError, AppResult, ValidationError}; @@ -78,7 +78,7 @@ pub fn render_template_input(input: &str, vars: &BTreeMap) -> St /// Returns an error when parsing or validation fails. pub fn apply_config_from_toml(input: &str) -> AppResult<()> { let config: ConfigFile = toml::from_str(input)?; - apply_config_to_defaults(&config) + apply_config_to_defaults(config) } /// Parses JSON config and applies it to defaults. @@ -88,7 +88,7 @@ pub fn apply_config_from_toml(input: &str) -> AppResult<()> { /// Returns an error when parsing or validation fails. pub fn apply_config_from_json(input: &[u8]) -> AppResult<()> { let config: ConfigFile = serde_json::from_slice(input)?; - apply_config_to_defaults(&config) + apply_config_to_defaults(config) } /// Parses a positive u64 string value. @@ -166,7 +166,7 @@ pub fn apply_load_config_input(load: LoadConfig) -> AppResult<()> { load: Some(load), ..ConfigFile::default() }; - apply_config_to_defaults(&config) + apply_config_to_defaults(config) } /// Parses a scenario config using default arguments. @@ -177,7 +177,13 @@ pub fn apply_load_config_input(load: LoadConfig) -> AppResult<()> { pub fn parse_scenario_config_input(config: &ScenarioConfig) -> AppResult<()> { BASE_MATCHES.with(|matches| { let args = TesterArgs::from_arg_matches(matches)?; - parse_scenario(config, &args).map(|_| ()) + let defaults = ScenarioDefaults::new( + args.url.clone(), + args.method, + args.data.clone(), + args.headers.clone(), + ); + parse_scenario(config, &defaults).map(|_| ()) }) } @@ -217,9 +223,9 @@ pub fn load_config_file_input(path: &std::path::Path) -> AppResult<()> { crate::config::load_config_file(path).map(|_| ()) } -fn apply_config_to_defaults(config: &ConfigFile) -> AppResult<()> { +fn apply_config_to_defaults(config: ConfigFile) -> AppResult<()> { BASE_MATCHES.with(|matches| { - let mut args = TesterArgs::from_arg_matches(matches)?; - apply_config(&mut args, matches, config) + let args = TesterArgs::from_arg_matches(matches)?; + apply_config(args, matches, config).map(|_| ()) }) } diff --git a/src/wasm_runtime/loader.rs b/src/wasm_runtime/loader.rs index 8292175..1ae9bfc 100644 --- a/src/wasm_runtime/loader.rs +++ b/src/wasm_runtime/loader.rs @@ -1,7 +1,7 @@ use wasmparser::{ExternalKind, ValType}; use crate::args::{Scenario, TesterArgs}; -use crate::config::apply::scenario::parse_scenario; +use crate::config::apply::scenario::{ScenarioDefaults, parse_scenario}; use crate::config::types::ScenarioConfig; use crate::error::{AppError, AppResult, ScriptError, WasmError}; @@ -92,7 +92,13 @@ pub(crate) fn load_scenario_from_wasm(script_path: &str, args: &TesterArgs) -> A .map_err(|err| AppError::script(WasmError::ScenarioJsonInvalid { source: err }))?; validate_wasm_scenario(&scenario_config)?; - parse_scenario(&scenario_config, args).map_err(|err| { + let defaults = ScenarioDefaults::new( + args.url.clone(), + args.method, + args.data.clone(), + args.headers.clone(), + ); + parse_scenario(&scenario_config, &defaults).map_err(|err| { if let AppError::Config(source) = err { AppError::script(ScriptError::ScenarioConfig { source }) } else {