From 0e72933e1cbe1645c71842acda0ea10c6ee6f522 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Mon, 7 Sep 2026 04:08:39 +0800 Subject: [PATCH 1/4] fix(config): reject malformed env overrides at startup instead of ignoring them Replace the per-module NODEDB_* env parsing (checkpoint, cluster, host_ports, numeric, timeseries, tls, wal, dispatch, helpers) with a single table-driven gate that validates every override and fails boot on a bad value rather than silently falling back to a default. Move loop intervals and the cluster join-retry policy (clone sweep, constraint reconcile, scope expiry, join attempts/backoff) out of ad hoc std::env::var reads scattered across background loops and into ServerConfig/MaintenanceTuning/ClusterSettings fields, so the same validated value reaches both production and the cluster test harness. --- .../node/lifecycle/spawn_full.rs | 17 +- nodedb-types/src/config/tuning/maintenance.rs | 55 ++++- nodedb/src/bootstrap/background_loops.rs | 15 +- nodedb/src/bootstrap/constraint_reconcile.rs | 6 +- nodedb/src/config/server/cluster.rs | 26 ++ nodedb/src/config/server/config.rs | 57 ++++- nodedb/src/config/server/domain.rs | 125 ++++++++++ nodedb/src/config/server/env/checkpoint.rs | 66 ----- nodedb/src/config/server/env/cluster.rs | 203 ---------------- nodedb/src/config/server/env/dispatch.rs | 67 ----- nodedb/src/config/server/env/helpers.rs | 121 --------- nodedb/src/config/server/env/host_ports.rs | 149 ------------ nodedb/src/config/server/env/mod.rs | 22 +- nodedb/src/config/server/env/numeric.rs | 77 ------ nodedb/src/config/server/env/parse.rs | 78 ++++++ .../src/config/server/env/rows/checkpoint.rs | 32 +++ nodedb/src/config/server/env/rows/cluster.rs | 124 ++++++++++ .../src/config/server/env/rows/host_ports.rs | 134 ++++++++++ .../src/config/server/env/rows/maintenance.rs | 55 +++++ nodedb/src/config/server/env/rows/mod.rs | 11 + .../config/server/env/rows/observability.rs | 100 ++++++++ nodedb/src/config/server/env/rows/sizing.rs | 74 ++++++ .../src/config/server/env/rows/timeseries.rs | 45 ++++ nodedb/src/config/server/env/rows/tls.rs | 126 ++++++++++ nodedb/src/config/server/env/rows/wal.rs | 54 +++++ nodedb/src/config/server/env/seed_nodes.rs | 44 ++++ nodedb/src/config/server/env/table.rs | 92 +++++++ nodedb/src/config/server/env/timeseries.rs | 28 --- nodedb/src/config/server/env/tls.rs | 17 -- nodedb/src/config/server/env/wal.rs | 72 ------ nodedb/src/config/server/mod.rs | 3 +- nodedb/src/config/server/observability.rs | 86 ------- nodedb/src/control/cluster/init.rs | 34 +-- nodedb/src/control/cluster/tls.rs | 2 + nodedb/src/control/security/scope/expiry.rs | 11 +- nodedb/src/main.rs | 12 +- nodedb/tests/config_env_cluster.rs | 127 ++++++++++ nodedb/tests/config_env_durability.rs | 96 ++++++++ nodedb/tests/config_env_listeners.rs | 229 ++++++++++++++++++ nodedb/tests/config_env_maintenance.rs | 58 +++++ nodedb/tests/config_env_observability.rs | 123 ++++++++++ nodedb/tests/config_env_sizing.rs | 136 +++++++++++ nodedb/tests/support/env_guard.rs | 70 ++++++ nodedb/tests/support/mod.rs | 1 + 44 files changed, 2123 insertions(+), 957 deletions(-) create mode 100644 nodedb/src/config/server/domain.rs delete mode 100644 nodedb/src/config/server/env/checkpoint.rs delete mode 100644 nodedb/src/config/server/env/cluster.rs delete mode 100644 nodedb/src/config/server/env/dispatch.rs delete mode 100644 nodedb/src/config/server/env/helpers.rs delete mode 100644 nodedb/src/config/server/env/host_ports.rs delete mode 100644 nodedb/src/config/server/env/numeric.rs create mode 100644 nodedb/src/config/server/env/parse.rs create mode 100644 nodedb/src/config/server/env/rows/checkpoint.rs create mode 100644 nodedb/src/config/server/env/rows/cluster.rs create mode 100644 nodedb/src/config/server/env/rows/host_ports.rs create mode 100644 nodedb/src/config/server/env/rows/maintenance.rs create mode 100644 nodedb/src/config/server/env/rows/mod.rs create mode 100644 nodedb/src/config/server/env/rows/observability.rs create mode 100644 nodedb/src/config/server/env/rows/sizing.rs create mode 100644 nodedb/src/config/server/env/rows/timeseries.rs create mode 100644 nodedb/src/config/server/env/rows/tls.rs create mode 100644 nodedb/src/config/server/env/rows/wal.rs create mode 100644 nodedb/src/config/server/env/seed_nodes.rs create mode 100644 nodedb/src/config/server/env/table.rs delete mode 100644 nodedb/src/config/server/env/timeseries.rs delete mode 100644 nodedb/src/config/server/env/tls.rs delete mode 100644 nodedb/src/config/server/env/wal.rs create mode 100644 nodedb/tests/config_env_cluster.rs create mode 100644 nodedb/tests/config_env_durability.rs create mode 100644 nodedb/tests/config_env_listeners.rs create mode 100644 nodedb/tests/config_env_maintenance.rs create mode 100644 nodedb/tests/config_env_observability.rs create mode 100644 nodedb/tests/config_env_sizing.rs create mode 100644 nodedb/tests/support/env_guard.rs diff --git a/nodedb-test-support/src/cluster_harness/node/lifecycle/spawn_full.rs b/nodedb-test-support/src/cluster_harness/node/lifecycle/spawn_full.rs index 882b4a19a..64cb89d60 100644 --- a/nodedb-test-support/src/cluster_harness/node/lifecycle/spawn_full.rs +++ b/nodedb-test-support/src/cluster_harness/node/lifecycle/spawn_full.rs @@ -170,6 +170,8 @@ impl TestClusterNode { login_attempts_per_user_per_min: 10, insecure_transport: true, log_compaction_threshold, + join_retry_max_attempts: 8, + join_retry_max_backoff_secs: 32, }; // Initialise the cluster using the pre-bound transport. @@ -328,7 +330,20 @@ impl TestClusterNode { // call; wire it directly so cluster tests exercise constraint delivery // to every replica's validator. Registered on `shared.loop_registry`, // so cluster shutdown stops it with the other loops. - nodedb::bootstrap::constraint_reconcile::spawn_constraint_reconcile(Arc::clone(&shared)); + // The interval comes from the same startup gate the server uses, so a + // test that sets `NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS` to suppress + // the loop is honoured here exactly as it is in production. Parsing it + // a second way here is what would let the two drift apart. + let mut reconcile_config = nodedb::ServerConfig::default(); + nodedb::config::server::apply_env_overrides(&mut reconcile_config) + .map_err(|e| format!("environment override rejected: {e}"))?; + nodedb::bootstrap::constraint_reconcile::spawn_constraint_reconcile( + Arc::clone(&shared), + reconcile_config + .tuning + .maintenance + .constraint_reconcile_interval_ms, + ); // Spawn the descriptor lease renewal loop on the same // shutdown channel as raft so cluster shutdown stops it diff --git a/nodedb-types/src/config/tuning/maintenance.rs b/nodedb-types/src/config/tuning/maintenance.rs index 5d604b7d1..a3046bdba 100644 --- a/nodedb-types/src/config/tuning/maintenance.rs +++ b/nodedb-types/src/config/tuning/maintenance.rs @@ -14,22 +14,63 @@ fn default_auto_analyze_min_mutations() -> u64 { 1_000 } +fn default_clone_sweep_interval_ms() -> u64 { + 30_000 +} + +fn default_constraint_reconcile_interval_ms() -> u64 { + 1_000 +} + +fn default_scope_expiry_interval_secs() -> u64 { + 60 +} + /// Tuning knobs for background maintenance triggered by user writes. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct MaintenanceTuning { /// Smallest mutation count that can trigger an automatic ANALYZE. /// - /// The trigger fires at `max(last_row_count / 10, this)`, so lowering it - /// makes a small collection refresh its statistics sooner, and raising it - /// trades planner accuracy for fewer background scans. + /// The trigger fires at `max(last_row_count / 10, this)`. Lowering it + /// refreshes a small collection's statistics sooner. Raising it trades + /// planner accuracy for fewer background scans. #[serde(default = "default_auto_analyze_min_mutations")] pub auto_analyze_min_mutations: u64, + + /// Interval between clone materializer sweeps, in milliseconds. + /// + /// The sweep progresses cloned collections from Shadowed to Materialized + /// without explicit DDL. Lowering it materializes clones sooner, at the + /// cost of more scan passes. + #[serde(default = "default_clone_sweep_interval_ms")] + pub clone_sweep_interval_ms: u64, + + /// Interval between CRDT constraint reconcile passes, in milliseconds. + /// + /// Each pass re-derives every collection's constraint set from the + /// catalog and replicates it to data-group replicas. Lowering it converges + /// an altered collection sooner, at the cost of catalog reads and Raft + /// proposals. + #[serde(default = "default_constraint_reconcile_interval_ms")] + pub constraint_reconcile_interval_ms: u64, + + /// Interval between scope grant expiry sweeps, in seconds. + /// + /// Each sweep executes the `ON EXPIRE` action of every expired grant. + /// `ScopeGrant::is_effective` already enforces expiry on every read, so + /// this loop only makes the outcome durable. 10 is the floor. Below it the + /// sweep costs more than the resolution it buys. + #[serde(default = "default_scope_expiry_interval_secs")] + pub scope_expiry_interval_secs: u64, } impl Default for MaintenanceTuning { fn default() -> Self { Self { auto_analyze_min_mutations: default_auto_analyze_min_mutations(), + clone_sweep_interval_ms: default_clone_sweep_interval_ms(), + constraint_reconcile_interval_ms: default_constraint_reconcile_interval_ms(), + scope_expiry_interval_secs: default_scope_expiry_interval_secs(), } } } @@ -46,6 +87,14 @@ mod tests { ); } + #[test] + fn new_loop_interval_defaults() { + let tuning = MaintenanceTuning::default(); + assert_eq!(tuning.clone_sweep_interval_ms, 30_000); + assert_eq!(tuning.constraint_reconcile_interval_ms, 1_000); + assert_eq!(tuning.scope_expiry_interval_secs, 60); + } + #[test] fn override_via_toml() { let parsed: MaintenanceTuning = diff --git a/nodedb/src/bootstrap/background_loops.rs b/nodedb/src/bootstrap/background_loops.rs index 3c7da8b40..17d95db11 100644 --- a/nodedb/src/bootstrap/background_loops.rs +++ b/nodedb/src/bootstrap/background_loops.rs @@ -288,10 +288,7 @@ pub fn spawn_background_loops( // materializer directly. { let shared_sweep = Arc::clone(shared); - let sweep_ms = std::env::var("NODEDB_CLONE_SWEEP_INTERVAL_MS") - .ok() - .and_then(|v| v.parse::().ok()) - .unwrap_or(30_000); + let sweep_ms = config.tuning.maintenance.clone_sweep_interval_ms; let sweep_interval = Duration::from_millis(sweep_ms); crate::control::shutdown::spawn_loop( &shared.loop_registry, @@ -339,13 +336,19 @@ pub fn spawn_background_loops( // That node re-derives each collection's constraint set from the // catalog and replicates it to every data-group replica's CRDT validator, // so a collection created/altered under any leader converges everywhere. - crate::bootstrap::constraint_reconcile::spawn_constraint_reconcile(Arc::clone(shared)); + crate::bootstrap::constraint_reconcile::spawn_constraint_reconcile( + Arc::clone(shared), + config.tuning.maintenance.constraint_reconcile_interval_ms, + ); info!("constraint reconcile loop running"); // Scope grant expiry sweep. Executes each expired grant's ON EXPIRE // action (hard revoke or downgrade to a lesser scope) through the // replicated propose path, so the change is durable and cluster-wide. - crate::control::security::scope::expiry::spawn_expiry_task(Arc::clone(shared)); + crate::control::security::scope::expiry::spawn_expiry_task( + Arc::clone(shared), + config.tuning.maintenance.scope_expiry_interval_secs, + ); // Cold tier task (if configured). if let Some(ref cold_settings) = config.cold_storage { diff --git a/nodedb/src/bootstrap/constraint_reconcile.rs b/nodedb/src/bootstrap/constraint_reconcile.rs index 9b47e0a21..d766099a5 100644 --- a/nodedb/src/bootstrap/constraint_reconcile.rs +++ b/nodedb/src/bootstrap/constraint_reconcile.rs @@ -46,7 +46,7 @@ const MAX_RECONCILE_PROPOSALS_PER_PASS: usize = 64; /// dispatches Control → Data proposes. The catalog read runs in /// `spawn_blocking` so a synchronous redb scan never stalls the reactor, and no /// lock is ever held across an `.await`. -pub fn spawn_constraint_reconcile(shared: Arc) { +pub fn spawn_constraint_reconcile(shared: Arc, interval_ms: u64) { // Clone for the task body so the original `shared` remains available to // borrow `loop_registry`/`shutdown` for the `spawn_loop` call itself. let task_shared = Arc::clone(&shared); @@ -62,10 +62,6 @@ pub fn spawn_constraint_reconcile(shared: Arc) { // accepted by Raft for each `(tenant, collection)`. Skipping equal // or older versions keeps steady-state ticks proposal-free. let mut delivered: HashMap<(TenantId, String), u64> = HashMap::new(); - let interval_ms = std::env::var("NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS") - .ok() - .and_then(|v| v.parse::().ok()) - .unwrap_or(1000); let mut tick = tokio::time::interval(Duration::from_millis(interval_ms)); loop { tokio::select! { diff --git a/nodedb/src/config/server/cluster.rs b/nodedb/src/config/server/cluster.rs index 078642073..e3ada31fd 100644 --- a/nodedb/src/config/server/cluster.rs +++ b/nodedb/src/config/server/cluster.rs @@ -109,6 +109,22 @@ pub struct ClusterSettings { /// `nodedb_cluster::ClusterConfig`. #[serde(default)] pub log_compaction_threshold: Option, + + /// Total join attempts before the join loop gives up. + /// + /// Production leaves this at its default. The integration test harness + /// lowers it so a join-retry path does not spend a minute sleeping. + /// Default: 8. + #[serde(default = "default_join_retry_max_attempts")] + pub join_retry_max_attempts: u32, + + /// Cap on the per-attempt join backoff delay, in seconds. + /// + /// Production leaves this at its default. The integration test harness + /// lowers it so a join-retry path does not spend a minute sleeping. + /// Default: 32. + #[serde(default = "default_join_retry_max_backoff_secs")] + pub join_retry_max_backoff_secs: u64, } /// Paths to on-disk PEM-encoded TLS credentials. @@ -152,6 +168,14 @@ fn default_login_attempts_per_user_per_min() -> u64 { 10 } +fn default_join_retry_max_attempts() -> u32 { + 8 +} + +fn default_join_retry_max_backoff_secs() -> u64 { + 32 +} + impl ClusterSettings { /// Validate cluster configuration. pub fn validate(&self) -> crate::Result<()> { @@ -215,6 +239,8 @@ mod tests { login_attempts_per_user_per_min: 10, insecure_transport: true, log_compaction_threshold: None, + join_retry_max_attempts: 8, + join_retry_max_backoff_secs: 32, } } diff --git a/nodedb/src/config/server/config.rs b/nodedb/src/config/server/config.rs index 2f2dcf27f..f1bd41838 100644 --- a/nodedb/src/config/server/config.rs +++ b/nodedb/src/config/server/config.rs @@ -139,7 +139,7 @@ impl ServerConfig { if let Some(ref jwt) = self.auth.jwt { jwt.validate()?; } - Ok(()) + super::domain::validate_domain(self) } /// Build a `SocketAddr` from the shared host and a port. @@ -322,4 +322,59 @@ mod tests { "unexpected error: {err}" ); } + + fn write_temp_config(name: &str, contents: &str) -> std::path::PathBuf { + let path = std::env::temp_dir().join(name); + std::fs::write(&path, contents).expect("write temp config"); + path + } + + /// The environment gate rejects a zero core count. A TOML file reaches the + /// same field without passing that gate, so the bound holds here too. + #[test] + fn from_file_rejects_zero_data_plane_cores() { + let path = write_temp_config( + "nodedb-domain-zero-cores.toml", + "[server]\ndata_plane_cores = 0\n", + ); + let err = ServerConfig::from_file(&path).unwrap_err(); + std::fs::remove_file(&path).ok(); + let msg = err.to_string(); + assert!(msg.contains("server.data_plane_cores"), "{msg}"); + assert!(msg.contains("positive integer"), "{msg}"); + } + + #[test] + fn from_file_rejects_scope_expiry_below_the_floor() { + let path = write_temp_config( + "nodedb-domain-expiry-floor.toml", + "[tuning.maintenance]\nscope_expiry_interval_secs = 5\n", + ); + let err = ServerConfig::from_file(&path).unwrap_err(); + std::fs::remove_file(&path).ok(); + let msg = err.to_string(); + assert!( + msg.contains("tuning.maintenance.scope_expiry_interval_secs"), + "{msg}" + ); + assert!(msg.contains("at least 10 seconds"), "{msg}"); + } + + #[test] + fn from_file_rejects_a_wal_write_buffer_under_the_floor() { + let path = write_temp_config( + "nodedb-domain-wal-buffer.toml", + "[tuning.wal]\nwrite_buffer_size = 4096\n", + ); + let err = ServerConfig::from_file(&path).unwrap_err(); + std::fs::remove_file(&path).ok(); + let msg = err.to_string(); + assert!(msg.contains("tuning.wal.write_buffer_size"), "{msg}"); + } + + /// Every shipped default satisfies every bound the gate enforces. + #[test] + fn the_compiled_defaults_are_in_domain() { + ServerConfig::default().validate().expect("defaults valid"); + } } diff --git a/nodedb/src/config/server/domain.rs b/nodedb/src/config/server/domain.rs new file mode 100644 index 000000000..a3d7abea2 --- /dev/null +++ b/nodedb/src/config/server/domain.rs @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Domain constraints on config values, checked whatever set them. +//! +//! The environment gate rejects an out-of-domain override and names the +//! variable. A TOML file reaches the same fields without passing that gate, so +//! [`validate_domain`] re-checks every constrained field on the loaded config. +//! Both paths read the bounds below, so neither can drift from the other. + +use super::ServerConfig; + +/// Smallest WAL write buffer the writer accepts. +pub(super) const MIN_WAL_WRITE_BUFFER_BYTES: usize = 64 * 1024; + +/// Smallest scope expiry sweep interval. +/// +/// `ScopeGrant::is_effective` already enforces expiry on every read, so a +/// shorter sweep costs more than the resolution it buys. +pub(super) const MIN_SCOPE_EXPIRY_SECS: u64 = 10; + +/// Rejects an endpoint that carries no `http://` or `https://` host. +pub(super) fn otlp_endpoint_has_host(raw: &str) -> bool { + raw.strip_prefix("http://") + .or_else(|| raw.strip_prefix("https://")) + .is_some_and(|host| !host.is_empty()) +} + +fn reject(field: &str, value: impl std::fmt::Display, expected: &str) -> crate::Error { + crate::Error::Config { + detail: format!("invalid value '{value}' for {field}: expected {expected}"), + } +} + +fn positive_u64(value: u64, field: &str) -> crate::Result<()> { + if value == 0 { + return Err(reject(field, value, "a positive integer")); + } + Ok(()) +} + +fn positive_usize(value: usize, field: &str) -> crate::Result<()> { + if value == 0 { + return Err(reject(field, value, "a positive integer")); + } + Ok(()) +} + +/// Checks every field the environment gate constrains, on the loaded config. +/// +/// A value set in TOML reaches the same field the gate guards. Skipping this +/// leaves the bound enforced on one of the two paths. +pub(super) fn validate_domain(config: &ServerConfig) -> crate::Result<()> { + positive_usize(config.server.data_plane_cores, "server.data_plane_cores")?; + + if config.tuning.wal.write_buffer_size < MIN_WAL_WRITE_BUFFER_BYTES { + return Err(reject( + "tuning.wal.write_buffer_size", + config.tuning.wal.write_buffer_size, + "a size of at least 64KiB", + )); + } + + positive_u64(config.checkpoint.interval_secs, "checkpoint.interval_secs")?; + positive_u64( + config.checkpoint.wal_segment_target_mb, + "checkpoint.wal_segment_target_mb", + )?; + + let ts = &config.tuning.timeseries; + positive_usize( + ts.memtable_budget_bytes, + "tuning.timeseries.memtable_budget_bytes", + )?; + positive_usize( + ts.memtable_hard_limit_bytes, + "tuning.timeseries.memtable_hard_limit_bytes", + )?; + positive_u64( + u64::from(ts.max_tag_cardinality), + "tuning.timeseries.max_tag_cardinality", + )?; + + let m = &config.tuning.maintenance; + positive_u64( + m.clone_sweep_interval_ms, + "tuning.maintenance.clone_sweep_interval_ms", + )?; + positive_u64( + m.constraint_reconcile_interval_ms, + "tuning.maintenance.constraint_reconcile_interval_ms", + )?; + if m.scope_expiry_interval_secs < MIN_SCOPE_EXPIRY_SECS { + return Err(reject( + "tuning.maintenance.scope_expiry_interval_secs", + m.scope_expiry_interval_secs, + "an interval of at least 10 seconds", + )); + } + + if let Some(cluster) = config.cluster.as_ref() { + positive_u64( + u64::from(cluster.join_retry_max_attempts), + "cluster.join_retry_max_attempts", + )?; + positive_u64( + cluster.join_retry_max_backoff_secs, + "cluster.join_retry_max_backoff_secs", + )?; + } + + let export = &config.observability.otlp.export; + positive_u64( + export.metrics_interval_secs, + "observability.otlp.export.metrics_interval_secs", + )?; + if export.enabled && !otlp_endpoint_has_host(&export.endpoint) { + return Err(reject( + "observability.otlp.export.endpoint", + &export.endpoint, + "an http:// or https:// endpoint URL", + )); + } + + Ok(()) +} diff --git a/nodedb/src/config/server/env/checkpoint.rs b/nodedb/src/config/server/env/checkpoint.rs deleted file mode 100644 index a25f8ef9d..000000000 --- a/nodedb/src/config/server/env/checkpoint.rs +++ /dev/null @@ -1,66 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! `NODEDB_CHECKPOINT_INTERVAL_SECS` / `NODEDB_WAL_SEGMENT_TARGET_MB` -//! overrides. -//! -//! Both drive the crash-recovery test harness: a short interval forces a -//! checkpoint cycle quickly, and a small segment target forces WAL rotation -//! so sealed segments exist for truncation to act on. - -use crate::config::server::ServerConfig; - -pub(super) fn apply_checkpoint_tuning(config: &mut ServerConfig) { - if let Ok(val) = std::env::var("NODEDB_CHECKPOINT_INTERVAL_SECS") { - match val.trim().parse::() { - Ok(secs) if secs > 0 => { - tracing::info!( - env_var = "NODEDB_CHECKPOINT_INTERVAL_SECS", - value = secs, - "environment variable override applied" - ); - config.checkpoint.interval_secs = secs; - } - Ok(secs) => { - tracing::warn!( - env_var = "NODEDB_CHECKPOINT_INTERVAL_SECS", - value = secs, - "ignoring value of 0 (checkpoint interval must be positive), using config value" - ); - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_CHECKPOINT_INTERVAL_SECS", - value = %val, - "ignoring malformed environment variable (expected u64 seconds), using config value" - ); - } - } - } - - if let Ok(val) = std::env::var("NODEDB_WAL_SEGMENT_TARGET_MB") { - match val.trim().parse::() { - Ok(mb) if mb > 0 => { - tracing::info!( - env_var = "NODEDB_WAL_SEGMENT_TARGET_MB", - value = mb, - "environment variable override applied" - ); - config.checkpoint.wal_segment_target_mb = mb; - } - Ok(mb) => { - tracing::warn!( - env_var = "NODEDB_WAL_SEGMENT_TARGET_MB", - value = mb, - "ignoring value of 0 (WAL segment target must be positive), using config value" - ); - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_WAL_SEGMENT_TARGET_MB", - value = %val, - "ignoring malformed environment variable (expected u64 MiB), using config value" - ); - } - } - } -} diff --git a/nodedb/src/config/server/env/cluster.rs b/nodedb/src/config/server/env/cluster.rs deleted file mode 100644 index 57d14e22f..000000000 --- a/nodedb/src/config/server/env/cluster.rs +++ /dev/null @@ -1,203 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! `NODEDB_NODE_ID` / `NODEDB_SEED_NODES` overrides — both are no-ops (with a -//! warning) unless a `[cluster]` section is already present in config. - -use std::net::SocketAddr; - -use crate::config::server::ServerConfig; - -pub(super) fn apply_cluster_overrides(config: &mut ServerConfig) { - if let Ok(val) = std::env::var("NODEDB_NODE_ID") { - match val.trim().parse::() { - Ok(node_id) => { - if let Some(cluster) = config.cluster.as_mut() { - tracing::info!( - env_var = "NODEDB_NODE_ID", - value = node_id, - "environment variable override applied" - ); - cluster.node_id = node_id; - } else { - tracing::warn!( - env_var = "NODEDB_NODE_ID", - value = node_id, - "NODEDB_NODE_ID is set but no [cluster] section is present in config; \ - ignoring (add a [cluster] section to enable cluster mode)" - ); - } - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_NODE_ID", - value = %val, - "ignoring malformed environment variable (expected u64), using config value" - ); - } - } - } - - if let Ok(val) = std::env::var("NODEDB_SEED_NODES") { - match parse_seed_nodes(&val) { - Ok(addrs) => { - if let Some(cluster) = config.cluster.as_mut() { - tracing::info!( - env_var = "NODEDB_SEED_NODES", - value = %val, - count = addrs.len(), - "environment variable override applied" - ); - cluster.seed_nodes = addrs; - } else { - tracing::warn!( - env_var = "NODEDB_SEED_NODES", - value = %val, - "NODEDB_SEED_NODES is set but no [cluster] section is present in config; \ - ignoring (add a [cluster] section to enable cluster mode)" - ); - } - } - Err(e) => { - tracing::warn!( - env_var = "NODEDB_SEED_NODES", - value = %val, - error = %e, - "ignoring malformed environment variable, using config value" - ); - } - } - } -} - -/// Parse a comma-separated list of `SocketAddr` strings. -/// -/// Returns `Ok(Vec)` if every entry parses successfully. -/// Returns `Err(bad_entry)` with the first entry that fails to parse, -/// so callers can log it and skip the entire override. -pub fn parse_seed_nodes(s: &str) -> crate::Result> { - let mut addrs = Vec::new(); - for entry in s.split(',') { - let entry = entry.trim(); - if entry.is_empty() { - continue; - } - match entry.parse::() { - Ok(addr) => addrs.push(addr), - Err(_) => { - return Err(crate::Error::Config { - detail: format!("invalid socket address: '{entry}'"), - }); - } - } - } - Ok(addrs) -} - -#[cfg(test)] -mod tests { - use super::super::dispatch::apply_env_overrides; - use super::*; - use crate::config::server::ClusterSettings; - - fn make_cluster(node_id: u64) -> ClusterSettings { - ClusterSettings { - node_id, - listen: "0.0.0.0:9400".parse().unwrap(), - seed_nodes: vec!["127.0.0.1:9400".parse().unwrap()], - num_groups: 4, - replication_factor: 3, - force_bootstrap: false, - tls: None, - max_active_sessions: 0, - login_attempts_per_ip_per_min: 30, - login_attempts_per_user_per_min: 10, - insecure_transport: false, - log_compaction_threshold: None, - } - } - - #[test] - fn env_cluster_overrides() { - // Always start clean. - unsafe { - std::env::remove_var("NODEDB_NODE_ID"); - std::env::remove_var("NODEDB_SEED_NODES"); - } - - // ── NODEDB_NODE_ID: valid value with cluster present → overrides node_id ── - - unsafe { std::env::set_var("NODEDB_NODE_ID", "42") }; - let mut cfg = ServerConfig { - cluster: Some(make_cluster(1)), - ..Default::default() - }; - apply_env_overrides(&mut cfg); - assert_eq!( - cfg.cluster.as_ref().unwrap().node_id, - 42, - "NODEDB_NODE_ID=42 should override node_id" - ); - unsafe { std::env::remove_var("NODEDB_NODE_ID") }; - - // ── NODEDB_NODE_ID: cluster absent → config.cluster stays None ── - - unsafe { std::env::set_var("NODEDB_NODE_ID", "99") }; - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert!( - cfg.cluster.is_none(), - "NODEDB_NODE_ID with no [cluster] section must not create cluster" - ); - unsafe { std::env::remove_var("NODEDB_NODE_ID") }; - - // ── NODEDB_NODE_ID: malformed value → node_id unchanged ── - - unsafe { std::env::set_var("NODEDB_NODE_ID", "not_a_number") }; - let mut cfg = ServerConfig { - cluster: Some(make_cluster(7)), - ..Default::default() - }; - apply_env_overrides(&mut cfg); - assert_eq!( - cfg.cluster.as_ref().unwrap().node_id, - 7, - "malformed NODEDB_NODE_ID must leave node_id unchanged" - ); - unsafe { std::env::remove_var("NODEDB_NODE_ID") }; - - // ── NODEDB_SEED_NODES: valid addresses with cluster present → overrides seed_nodes ── - - unsafe { std::env::set_var("NODEDB_SEED_NODES", "10.0.0.1:9400,10.0.0.2:9400") }; - let mut cfg = ServerConfig { - cluster: Some(make_cluster(1)), - ..Default::default() - }; - apply_env_overrides(&mut cfg); - let seeds = &cfg.cluster.as_ref().unwrap().seed_nodes; - assert_eq!(seeds.len(), 2, "two seed addresses should be applied"); - assert_eq!(seeds[0].to_string(), "10.0.0.1:9400"); - assert_eq!(seeds[1].to_string(), "10.0.0.2:9400"); - unsafe { std::env::remove_var("NODEDB_SEED_NODES") }; - - // ── NODEDB_SEED_NODES: malformed entry → seed_nodes unchanged (no partial apply) ── - - unsafe { std::env::set_var("NODEDB_SEED_NODES", "10.0.0.1:9400,garbage") }; - let existing_seed: SocketAddr = "192.168.1.1:9400".parse().unwrap(); - let mut cfg = ServerConfig { - cluster: Some(ClusterSettings { - seed_nodes: vec![existing_seed], - ..make_cluster(1) - }), - ..Default::default() - }; - apply_env_overrides(&mut cfg); - let seeds = &cfg.cluster.as_ref().unwrap().seed_nodes; - assert_eq!( - seeds.len(), - 1, - "malformed NODEDB_SEED_NODES must not partially apply" - ); - assert_eq!(seeds[0], existing_seed); - unsafe { std::env::remove_var("NODEDB_SEED_NODES") }; - } -} diff --git a/nodedb/src/config/server/env/dispatch.rs b/nodedb/src/config/server/env/dispatch.rs deleted file mode 100644 index 2f3166e7d..000000000 --- a/nodedb/src/config/server/env/dispatch.rs +++ /dev/null @@ -1,67 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -use super::checkpoint::apply_checkpoint_tuning; -use super::cluster::apply_cluster_overrides; -use super::host_ports::apply_host_and_ports; -use super::numeric::apply_numeric_settings; -use super::timeseries::apply_timeseries_overrides; -use super::tls::apply_tls_overrides; -use super::wal::apply_wal_tuning; -use crate::config::server::ServerConfig; - -/// Apply environment variable overrides to a loaded `ServerConfig`. -/// -/// Priority order: env var > TOML value > compiled default. -/// -/// Handled variables: -/// - `NODEDB_HOST` — overrides `config.host` (bind address, e.g., `0.0.0.0`) -/// - `NODEDB_SYNC_HOST` — overrides `config.sync_host` (sync listener only; loopback-only) -/// - `NODEDB_PORT_NATIVE` — overrides `config.ports.native` (default 6433) -/// - `NODEDB_PORT_PGWIRE` — overrides `config.ports.pgwire` (default 6432) -/// - `NODEDB_PORT_HTTP` — overrides `config.ports.http` (default 6480) -/// - `NODEDB_PORT_SYNC` — overrides `config.ports.sync` (default 9090) -/// - `NODEDB_PORT_RESP` — overrides `config.ports.resp` (set to enable RESP) -/// - `NODEDB_PORT_ILP` — overrides `config.ports.ilp` (set to enable ILP) -/// - `NODEDB_DATA_DIR` — overrides `config.data_dir` -/// - `NODEDB_MEMORY_LIMIT` — overrides `config.memory_limit` -/// - `NODEDB_DATA_PLANE_CORES` — overrides `config.data_plane_cores` (parse as usize) -/// - `NODEDB_MAX_CONNECTIONS` — overrides `config.max_connections` (parse as usize) -/// - `NODEDB_LOG_FORMAT` — overrides `config.log_format` ("text" or "json") -/// - `NODEDB_TLS_NATIVE` — enable/disable TLS on native protocol ("true"/"false") -/// - `NODEDB_TLS_PGWIRE` — enable/disable TLS on pgwire ("true"/"false") -/// - `NODEDB_TLS_HTTP` — enable/disable TLS on HTTP ("true"/"false") -/// - `NODEDB_TLS_RESP` — enable/disable TLS on RESP ("true"/"false") -/// - `NODEDB_TLS_ILP` — enable/disable TLS on ILP ("true"/"false") -/// - `NODEDB_NODE_ID` — overrides `config.cluster.node_id` (parse as u64) -/// - `NODEDB_SEED_NODES` — overrides `config.cluster.seed_nodes` -/// (comma-separated `SocketAddr` list) -/// - `NODEDB_CHECKPOINT_INTERVAL_SECS` — overrides `config.checkpoint.interval_secs` -/// (parse as u64 seconds; 0 is rejected) -/// - `NODEDB_WAL_SEGMENT_TARGET_MB` — overrides `config.checkpoint.wal_segment_target_mb` -/// (parse as u64 MiB; 0 is rejected) -/// - `NODEDB_WAL_DIRECT_IO` — overrides `config.tuning.wal.direct_io` -/// ("true"/"false"; default true) -/// - `NODEDB_TS_MEMTABLE_BUDGET_BYTES` — overrides -/// `config.tuning.timeseries.memtable_budget_bytes` (parse as usize; 0 is rejected) -/// - `NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES` — overrides -/// `config.tuning.timeseries.memtable_hard_limit_bytes` (parse as usize; 0 is rejected) -/// - `NODEDB_TS_MAX_TAG_CARDINALITY` — overrides -/// `config.tuning.timeseries.max_tag_cardinality` (parse as u32; 0 is rejected) -/// -/// `NODEDB_CONFIG` (config file path) is handled upstream in `main.rs` -/// before this function is called, so it is not processed here. -/// -/// `NODEDB_SUPERUSER_PASSWORD` is intentionally absent from this list. It is -/// handled separately by `crate::config::auth::AuthConfig::resolve_superuser_password()` -/// (called from `main.rs`) so that the value is never passed through logging -/// code paths or stored in `ServerConfig` where it could appear in debug output. -pub fn apply_env_overrides(config: &mut ServerConfig) { - apply_host_and_ports(config); - apply_cluster_overrides(config); - apply_numeric_settings(config); - apply_tls_overrides(config); - apply_wal_tuning(config); - apply_checkpoint_tuning(config); - apply_timeseries_overrides(config); - super::super::observability::apply_observability_env(&mut config.observability); -} diff --git a/nodedb/src/config/server/env/helpers.rs b/nodedb/src/config/server/env/helpers.rs deleted file mode 100644 index 9caf3cda3..000000000 --- a/nodedb/src/config/server/env/helpers.rs +++ /dev/null @@ -1,121 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! Generic single-variable env parsing helpers shared across the override -//! sections. Each captures one recurring shape (port, positive integer, -//! optional port, bool) so the info/warn wording stays identical across the -//! knobs that share it instead of letting copy-pasted blocks drift. - -/// Parse a u16 port from an env var into a required field. -pub(super) fn apply_port_env(var: &str, target: &mut u16) { - if let Ok(val) = std::env::var(var) { - match val.trim().parse::() { - Ok(port) => { - tracing::info!( - env_var = var, - value = port, - "environment variable override applied" - ); - *target = port; - } - Err(_) => { - tracing::warn!( - env_var = var, - value = %val, - "ignoring malformed environment variable (expected port number), using config value" - ); - } - } - } -} - -/// Parse a strictly-positive integer from an env var into a required field. -/// -/// Zero is rejected rather than applied: every knob routed through here is a -/// budget or a ceiling, and zero would mean "admit nothing", which is a -/// misconfiguration to warn about rather than a value to honour. -pub(super) fn apply_positive_env(var: &str, target: &mut T) -where - T: std::str::FromStr + PartialEq + From + std::fmt::Display + Copy, -{ - let Ok(val) = std::env::var(var) else { - return; - }; - match val.trim().parse::() { - Ok(parsed) if parsed != T::from(0) => { - tracing::info!( - env_var = var, - value = %parsed, - "environment variable override applied" - ); - *target = parsed; - } - Ok(zero) => { - tracing::warn!( - env_var = var, - value = %zero, - "ignoring value of 0 (must be positive), using config value" - ); - } - Err(_) => { - tracing::warn!( - env_var = var, - value = %val, - "ignoring malformed environment variable (expected a positive integer), using config value" - ); - } - } -} - -/// Parse a u16 port from an env var into an optional field (enables the listener). -pub(super) fn apply_optional_port_env(var: &str, target: &mut Option) { - if let Ok(val) = std::env::var(var) { - match val.trim().parse::() { - Ok(port) => { - tracing::info!( - env_var = var, - value = port, - "environment variable override applied" - ); - *target = Some(port); - } - Err(_) => { - tracing::warn!( - env_var = var, - value = %val, - "ignoring malformed environment variable (expected port number), using config value" - ); - } - } - } -} - -/// Parse a boolean env var ("true"/"false") into a bool field. -pub(super) fn apply_bool_env(var: &str, target: &mut bool) { - if let Ok(val) = std::env::var(var) { - match val.trim().to_lowercase().as_str() { - "true" | "1" | "yes" => { - tracing::info!( - env_var = var, - value = true, - "environment variable override applied" - ); - *target = true; - } - "false" | "0" | "no" => { - tracing::info!( - env_var = var, - value = false, - "environment variable override applied" - ); - *target = false; - } - _ => { - tracing::warn!( - env_var = var, - value = %val, - "ignoring malformed environment variable (expected true/false), using config value" - ); - } - } - } -} diff --git a/nodedb/src/config/server/env/host_ports.rs b/nodedb/src/config/server/env/host_ports.rs deleted file mode 100644 index 8f4ec7c15..000000000 --- a/nodedb/src/config/server/env/host_ports.rs +++ /dev/null @@ -1,149 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! `NODEDB_HOST` / `NODEDB_PORT_*` / `NODEDB_DATA_DIR` / `NODEDB_MEMORY_LIMIT` -//! overrides — the bind address and per-protocol listener configuration. - -use std::net::IpAddr; - -use super::helpers::{apply_optional_port_env, apply_port_env}; -use super::memory_size::parse_memory_size; -use crate::config::server::ServerConfig; - -pub(super) fn apply_host_and_ports(config: &mut ServerConfig) { - if let Ok(val) = std::env::var("NODEDB_HOST") { - match val.trim().parse::() { - Ok(ip) => { - tracing::info!(env_var = "NODEDB_HOST", value = %val, "environment variable override applied"); - config.server.host = ip; - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_HOST", - value = %val, - "ignoring malformed environment variable (expected IP address), using config value" - ); - } - } - } - - // Separate from `NODEDB_HOST`: sync is loopback-only, so this selects a - // different loopback (`::1`, 127.0.0.2), not a shared routable address. - if let Ok(val) = std::env::var("NODEDB_SYNC_HOST") { - match val.trim().parse::() { - Ok(ip) => { - tracing::info!(env_var = "NODEDB_SYNC_HOST", value = %val, "environment variable override applied"); - config.server.sync_host = Some(ip); - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_SYNC_HOST", - value = %val, - "ignoring malformed environment variable (expected IP address), using config value" - ); - } - } - } - - apply_port_env("NODEDB_PORT_NATIVE", &mut config.server.ports.native); - apply_port_env("NODEDB_PORT_PGWIRE", &mut config.server.ports.pgwire); - apply_port_env("NODEDB_PORT_HTTP", &mut config.server.ports.http); - apply_port_env("NODEDB_PORT_SYNC", &mut config.server.ports.sync); - apply_optional_port_env("NODEDB_PORT_RESP", &mut config.server.ports.resp); - apply_optional_port_env("NODEDB_PORT_ILP", &mut config.server.ports.ilp); - - if let Ok(val) = std::env::var("NODEDB_DATA_DIR") { - let path = std::path::PathBuf::from(&val); - tracing::info!( - env_var = "NODEDB_DATA_DIR", - value = %val, - "environment variable override applied" - ); - config.server.data_dir = path; - } - - if let Ok(val) = std::env::var("NODEDB_MEMORY_LIMIT") { - match parse_memory_size(&val) { - Ok(bytes) => { - tracing::info!( - env_var = "NODEDB_MEMORY_LIMIT", - value = %val, - bytes, - "environment variable override applied" - ); - config.server.memory_limit = bytes; - } - Err(e) => { - tracing::warn!( - env_var = "NODEDB_MEMORY_LIMIT", - value = %val, - error = %e, - "ignoring malformed environment variable, using config value" - ); - } - } - } -} - -#[cfg(test)] -mod tests { - use super::super::dispatch::apply_env_overrides; - use super::*; - - #[test] - fn env_data_dir_override() { - unsafe { std::env::set_var("NODEDB_DATA_DIR", "/tmp/test-nodedb") }; - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert_eq!( - cfg.server.data_dir, - std::path::PathBuf::from("/tmp/test-nodedb") - ); - unsafe { std::env::remove_var("NODEDB_DATA_DIR") }; - } - - /// Tests valid and malformed `NODEDB_MEMORY_LIMIT` sequentially to avoid - /// env-var races (env vars are process-global, Rust tests run in parallel). - #[test] - fn env_memory_limit_overrides() { - // ── Valid value → overrides memory_limit ── - unsafe { std::env::set_var("NODEDB_MEMORY_LIMIT", "2GiB") }; - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert_eq!(cfg.server.memory_limit, 2 * 1024 * 1024 * 1024); - - // ── Malformed value → memory_limit unchanged ── - unsafe { std::env::set_var("NODEDB_MEMORY_LIMIT", "notanumber") }; - let mut cfg = ServerConfig::default(); - let before = cfg.server.memory_limit; - apply_env_overrides(&mut cfg); - assert_eq!( - cfg.server.memory_limit, before, - "malformed value must not change config" - ); - - unsafe { std::env::remove_var("NODEDB_MEMORY_LIMIT") }; - } - - /// Tests valid and malformed `NODEDB_PORT_SYNC` sequentially to avoid - /// env-var races (env vars are process-global, Rust tests run in parallel). - #[test] - fn env_sync_port_overrides() { - // ── Valid value → overrides ports.sync ── - unsafe { std::env::set_var("NODEDB_PORT_SYNC", "19090") }; - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert_eq!(cfg.server.ports.sync, 19090); - - // ── Malformed value → ports.sync unchanged ── - unsafe { std::env::set_var("NODEDB_PORT_SYNC", "notaport") }; - let mut cfg = ServerConfig::default(); - let before = cfg.server.ports.sync; - apply_env_overrides(&mut cfg); - assert_eq!( - cfg.server.ports.sync, before, - "malformed value must not change config" - ); - - unsafe { std::env::remove_var("NODEDB_PORT_SYNC") }; - } -} diff --git a/nodedb/src/config/server/env/mod.rs b/nodedb/src/config/server/env/mod.rs index acf12f532..c5a93704c 100644 --- a/nodedb/src/config/server/env/mod.rs +++ b/nodedb/src/config/server/env/mod.rs @@ -1,20 +1,14 @@ // SPDX-License-Identifier: BUSL-1.1 -//! Environment variable overrides for `ServerConfig`, split by concern. -//! `dispatch::apply_env_overrides` is the public entry point; every other -//! submodule here handles one section of the override surface. +//! Environment variable overrides for `ServerConfig`: a table-driven +//! startup gate. See `table::apply_env_overrides`. -mod checkpoint; -mod cluster; -mod dispatch; -mod helpers; -mod host_ports; mod memory_size; -mod numeric; -mod timeseries; -mod tls; -mod wal; +mod parse; +mod rows; +mod seed_nodes; +mod table; -pub use cluster::parse_seed_nodes; -pub use dispatch::apply_env_overrides; pub use memory_size::parse_memory_size; +pub use seed_nodes::parse_seed_nodes; +pub use table::apply_env_overrides; diff --git a/nodedb/src/config/server/env/numeric.rs b/nodedb/src/config/server/env/numeric.rs deleted file mode 100644 index 09bb5a0a1..000000000 --- a/nodedb/src/config/server/env/numeric.rs +++ /dev/null @@ -1,77 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! `NODEDB_DATA_PLANE_CORES` / `NODEDB_MAX_CONNECTIONS` / `NODEDB_LOG_FORMAT` -//! overrides. - -use crate::config::server::{LogFormat, ServerConfig}; - -pub(super) fn apply_numeric_settings(config: &mut ServerConfig) { - if let Ok(val) = std::env::var("NODEDB_DATA_PLANE_CORES") { - match val.trim().parse::() { - Ok(cores) => { - tracing::info!( - env_var = "NODEDB_DATA_PLANE_CORES", - value = cores, - "environment variable override applied" - ); - config.server.data_plane_cores = cores; - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_DATA_PLANE_CORES", - value = %val, - "ignoring malformed environment variable (expected usize), using config value" - ); - } - } - } - - if let Ok(val) = std::env::var("NODEDB_MAX_CONNECTIONS") { - match val.trim().parse::() { - Ok(n) => { - tracing::info!( - env_var = "NODEDB_MAX_CONNECTIONS", - value = n, - "environment variable override applied" - ); - config.server.max_connections = n; - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_MAX_CONNECTIONS", - value = %val, - "ignoring malformed environment variable (expected usize), using config value" - ); - } - } - } - - if let Ok(val) = std::env::var("NODEDB_LOG_FORMAT") { - let normalised = val.trim().to_lowercase(); - match normalised.as_str() { - "text" => { - tracing::info!( - env_var = "NODEDB_LOG_FORMAT", - value = "text", - "environment variable override applied" - ); - config.server.log_format = LogFormat::Text; - } - "json" => { - tracing::info!( - env_var = "NODEDB_LOG_FORMAT", - value = "json", - "environment variable override applied" - ); - config.server.log_format = LogFormat::Json; - } - _ => { - tracing::warn!( - env_var = "NODEDB_LOG_FORMAT", - value = %val, - "ignoring malformed environment variable (expected \"text\" or \"json\"), using config value" - ); - } - } - } -} diff --git a/nodedb/src/config/server/env/parse.rs b/nodedb/src/config/server/env/parse.rs new file mode 100644 index 000000000..bd09d7a67 --- /dev/null +++ b/nodedb/src/config/server/env/parse.rs @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Shared value parsers for `NODEDB_*` env row `apply` functions. +//! +//! Each returns the expectation text as its `Err`. A row's violation message +//! then carries what the process needed, not a parser-specific string that +//! drifts from the row table. + +use std::net::{IpAddr, SocketAddr}; + +pub(super) fn parse_ip(raw: &str) -> Result { + raw.trim().parse::().map_err(|_| "an IP address") +} + +/// Parses a listener port. Zero is out of domain: no listener binds to it. +pub(super) fn parse_port(raw: &str) -> Result { + match raw.trim().parse::() { + Ok(0) | Err(_) => Err("a port number (1-65535)"), + Ok(port) => Ok(port), + } +} + +pub(super) fn parse_socket_addr( + raw: &str, + expected: &'static str, +) -> Result { + raw.trim().parse::().map_err(|_| expected) +} + +/// Accepts `true`/`1`/`yes` and `false`/`0`/`no`, case-insensitive. Every +/// boolean row shares this vocabulary, observability toggles included. +pub(super) fn parse_bool_lenient(raw: &str) -> Result { + match raw.trim().to_ascii_lowercase().as_str() { + "true" | "1" | "yes" => Ok(true), + "false" | "0" | "no" => Ok(false), + _ => Err("true or false"), + } +} + +pub(super) fn parse_usize_positive(raw: &str) -> Result { + match raw.trim().parse::() { + Ok(n) if n > 0 => Ok(n), + _ => Err("a positive integer"), + } +} + +pub(super) fn parse_usize_nonneg(raw: &str) -> Result { + raw.trim() + .parse::() + .map_err(|_| "a non-negative integer") +} + +pub(super) fn parse_u32_positive(raw: &str) -> Result { + match raw.trim().parse::() { + Ok(n) if n > 0 => Ok(n), + _ => Err("a positive integer"), + } +} + +pub(super) fn parse_u64_positive(raw: &str) -> Result { + match raw.trim().parse::() { + Ok(n) if n > 0 => Ok(n), + _ => Err("a positive integer"), + } +} + +/// Parses a `u64` no smaller than `min`, for the one row (scope-expiry) +/// whose floor sits above zero. +pub(super) fn parse_u64_at_least( + raw: &str, + min: u64, + expected: &'static str, +) -> Result { + match raw.trim().parse::() { + Ok(n) if n >= min => Ok(n), + _ => Err(expected), + } +} diff --git a/nodedb/src/config/server/env/rows/checkpoint.rs b/nodedb/src/config/server/env/rows/checkpoint.rs new file mode 100644 index 000000000..a9dd692f1 --- /dev/null +++ b/nodedb/src/config/server/env/rows/checkpoint.rs @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_CHECKPOINT_INTERVAL_SECS` / `NODEDB_WAL_SEGMENT_TARGET_MB` +//! overrides. + +use crate::config::server::ServerConfig; + +use super::super::parse::parse_u64_positive; +use super::super::table::EnvRow; + +fn apply_checkpoint_interval(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.checkpoint.interval_secs = parse_u64_positive(raw)?; + Ok(()) +} + +fn apply_wal_segment_target_mb(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.checkpoint.wal_segment_target_mb = parse_u64_positive(raw)?; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_CHECKPOINT_INTERVAL_SECS", + apply: apply_checkpoint_interval, + redact: false, + }, + EnvRow { + name: "NODEDB_WAL_SEGMENT_TARGET_MB", + apply: apply_wal_segment_target_mb, + redact: false, + }, +]; diff --git a/nodedb/src/config/server/env/rows/cluster.rs b/nodedb/src/config/server/env/rows/cluster.rs new file mode 100644 index 000000000..a53f82d3d --- /dev/null +++ b/nodedb/src/config/server/env/rows/cluster.rs @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_NODE_ID` / `NODEDB_SEED_NODES` / `NODEDB_JOIN_RETRY_MAX_ATTEMPTS` +//! / `NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS` overrides. +//! +//! Every row here needs a `[cluster]` section already in the loaded config. +//! The process cannot invent a cluster identity for itself. + +use crate::config::server::ServerConfig; + +use super::super::parse::{parse_u32_positive, parse_u64_positive}; +use super::super::seed_nodes::parse_seed_nodes; +use super::super::table::EnvRow; + +const NO_CLUSTER: &str = "a [cluster] section in the config file"; + +fn apply_node_id(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + let node_id = raw.trim().parse::().map_err(|_| "a u64 node id")?; + let cluster = config.cluster.as_mut().ok_or(NO_CLUSTER)?; + cluster.node_id = node_id; + Ok(()) +} + +fn apply_seed_nodes(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + let addrs = parse_seed_nodes(raw)?; + let cluster = config.cluster.as_mut().ok_or(NO_CLUSTER)?; + cluster.seed_nodes = addrs; + Ok(()) +} + +fn apply_join_retry_max_attempts(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + let attempts = parse_u32_positive(raw)?; + let cluster = config.cluster.as_mut().ok_or(NO_CLUSTER)?; + cluster.join_retry_max_attempts = attempts; + Ok(()) +} + +fn apply_join_retry_max_backoff_secs( + config: &mut ServerConfig, + raw: &str, +) -> Result<(), &'static str> { + let secs = parse_u64_positive(raw)?; + let cluster = config.cluster.as_mut().ok_or(NO_CLUSTER)?; + cluster.join_retry_max_backoff_secs = secs; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_NODE_ID", + apply: apply_node_id, + redact: false, + }, + EnvRow { + name: "NODEDB_SEED_NODES", + apply: apply_seed_nodes, + redact: false, + }, + EnvRow { + name: "NODEDB_JOIN_RETRY_MAX_ATTEMPTS", + apply: apply_join_retry_max_attempts, + redact: false, + }, + EnvRow { + name: "NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS", + apply: apply_join_retry_max_backoff_secs, + redact: false, + }, +]; + +#[cfg(test)] +mod tests { + use super::super::super::apply_env_overrides; + use super::*; + use crate::config::server::ClusterSettings; + + fn make_cluster(node_id: u64) -> ClusterSettings { + ClusterSettings { + node_id, + listen: "0.0.0.0:9400".parse().expect("listen address"), + seed_nodes: vec!["127.0.0.1:9400".parse().expect("seed address")], + num_groups: 4, + replication_factor: 3, + force_bootstrap: false, + tls: None, + max_active_sessions: 0, + login_attempts_per_ip_per_min: 30, + login_attempts_per_user_per_min: 10, + insecure_transport: false, + log_compaction_threshold: None, + join_retry_max_attempts: 8, + join_retry_max_backoff_secs: 32, + } + } + + #[test] + fn env_cluster_overrides() { + unsafe { + std::env::set_var("NODEDB_NODE_ID", "42"); + std::env::set_var("NODEDB_SEED_NODES", "10.0.0.1:9400,10.0.0.2:9400"); + } + let mut cfg = ServerConfig { + cluster: Some(make_cluster(1)), + ..Default::default() + }; + apply_env_overrides(&mut cfg).expect("valid cluster overrides must apply"); + let cluster = cfg.cluster.as_ref().expect("cluster section"); + assert_eq!( + cluster.node_id, 42, + "NODEDB_NODE_ID=42 must override node_id" + ); + assert_eq!( + cluster.seed_nodes.len(), + 2, + "both seed addresses must apply" + ); + assert_eq!(cluster.seed_nodes[0].to_string(), "10.0.0.1:9400"); + assert_eq!(cluster.seed_nodes[1].to_string(), "10.0.0.2:9400"); + unsafe { + std::env::remove_var("NODEDB_NODE_ID"); + std::env::remove_var("NODEDB_SEED_NODES"); + } + } +} diff --git a/nodedb/src/config/server/env/rows/host_ports.rs b/nodedb/src/config/server/env/rows/host_ports.rs new file mode 100644 index 000000000..b9a1bbdcb --- /dev/null +++ b/nodedb/src/config/server/env/rows/host_ports.rs @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_HOST` / `NODEDB_SYNC_HOST` / `NODEDB_PORT_*` / `NODEDB_DATA_DIR` +//! overrides — bind address, per-protocol listener ports, and the on-disk +//! data directory. + +use std::path::PathBuf; + +use crate::config::server::ServerConfig; + +use super::super::parse::{parse_ip, parse_port}; +use super::super::table::EnvRow; + +fn apply_host(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.host = parse_ip(raw)?; + Ok(()) +} + +/// Separate from `NODEDB_HOST`: sync is loopback-only, so this selects a +/// different loopback (`::1`, `127.0.0.2`), not a shared routable address. +fn apply_sync_host(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.sync_host = Some(parse_ip(raw)?); + Ok(()) +} + +fn apply_port_native(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.ports.native = parse_port(raw)?; + Ok(()) +} + +fn apply_port_pgwire(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.ports.pgwire = parse_port(raw)?; + Ok(()) +} + +fn apply_port_http(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.ports.http = parse_port(raw)?; + Ok(()) +} + +fn apply_port_sync(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.ports.sync = parse_port(raw)?; + Ok(()) +} + +fn apply_port_resp(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.ports.resp = Some(parse_port(raw)?); + Ok(()) +} + +fn apply_port_ilp(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.ports.ilp = Some(parse_port(raw)?); + Ok(()) +} + +fn apply_data_dir(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.data_dir = PathBuf::from(raw); + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_HOST", + apply: apply_host, + redact: false, + }, + EnvRow { + name: "NODEDB_SYNC_HOST", + apply: apply_sync_host, + redact: false, + }, + EnvRow { + name: "NODEDB_PORT_NATIVE", + apply: apply_port_native, + redact: false, + }, + EnvRow { + name: "NODEDB_PORT_PGWIRE", + apply: apply_port_pgwire, + redact: false, + }, + EnvRow { + name: "NODEDB_PORT_HTTP", + apply: apply_port_http, + redact: false, + }, + EnvRow { + name: "NODEDB_PORT_SYNC", + apply: apply_port_sync, + redact: false, + }, + EnvRow { + name: "NODEDB_PORT_RESP", + apply: apply_port_resp, + redact: false, + }, + EnvRow { + name: "NODEDB_PORT_ILP", + apply: apply_port_ilp, + redact: false, + }, + EnvRow { + name: "NODEDB_DATA_DIR", + apply: apply_data_dir, + redact: false, + }, +]; + +#[cfg(test)] +mod tests { + use super::super::super::apply_env_overrides; + use super::*; + + #[test] + fn env_data_dir_override() { + unsafe { std::env::set_var("NODEDB_DATA_DIR", "/tmp/test-nodedb") }; + let mut cfg = ServerConfig::default(); + apply_env_overrides(&mut cfg).expect("a valid data dir must apply"); + assert_eq!( + cfg.server.data_dir, + std::path::PathBuf::from("/tmp/test-nodedb") + ); + unsafe { std::env::remove_var("NODEDB_DATA_DIR") }; + } + + #[test] + fn env_sync_port_overrides() { + unsafe { std::env::set_var("NODEDB_PORT_SYNC", "19090") }; + let mut cfg = ServerConfig::default(); + apply_env_overrides(&mut cfg).expect("a valid sync port must apply"); + assert_eq!(cfg.server.ports.sync, 19090); + unsafe { std::env::remove_var("NODEDB_PORT_SYNC") }; + } +} diff --git a/nodedb/src/config/server/env/rows/maintenance.rs b/nodedb/src/config/server/env/rows/maintenance.rs new file mode 100644 index 000000000..7383646bc --- /dev/null +++ b/nodedb/src/config/server/env/rows/maintenance.rs @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_CLONE_SWEEP_INTERVAL_MS` / `NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS` +//! / `NODEDB_SCOPE_EXPIRY_INTERVAL_SECS` overrides — background maintenance +//! loop intervals. + +use crate::config::server::ServerConfig; + +use super::super::parse::{parse_u64_at_least, parse_u64_positive}; +use super::super::table::EnvRow; +use crate::config::server::domain::MIN_SCOPE_EXPIRY_SECS; + +/// Below 10 seconds the sweep costs more than the resolution it buys. +fn apply_clone_sweep_interval_ms(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.tuning.maintenance.clone_sweep_interval_ms = parse_u64_positive(raw)?; + Ok(()) +} + +fn apply_constraint_reconcile_interval_ms( + config: &mut ServerConfig, + raw: &str, +) -> Result<(), &'static str> { + config.tuning.maintenance.constraint_reconcile_interval_ms = parse_u64_positive(raw)?; + Ok(()) +} + +fn apply_scope_expiry_interval_secs( + config: &mut ServerConfig, + raw: &str, +) -> Result<(), &'static str> { + config.tuning.maintenance.scope_expiry_interval_secs = parse_u64_at_least( + raw, + MIN_SCOPE_EXPIRY_SECS, + "an interval of at least 10 seconds", + )?; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_CLONE_SWEEP_INTERVAL_MS", + apply: apply_clone_sweep_interval_ms, + redact: false, + }, + EnvRow { + name: "NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS", + apply: apply_constraint_reconcile_interval_ms, + redact: false, + }, + EnvRow { + name: "NODEDB_SCOPE_EXPIRY_INTERVAL_SECS", + apply: apply_scope_expiry_interval_secs, + redact: false, + }, +]; diff --git a/nodedb/src/config/server/env/rows/mod.rs b/nodedb/src/config/server/env/rows/mod.rs new file mode 100644 index 000000000..d7820eacb --- /dev/null +++ b/nodedb/src/config/server/env/rows/mod.rs @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: BUSL-1.1 + +pub(super) mod checkpoint; +pub(super) mod cluster; +pub(super) mod host_ports; +pub(super) mod maintenance; +pub(super) mod observability; +pub(super) mod sizing; +pub(super) mod timeseries; +pub(super) mod tls; +pub(super) mod wal; diff --git a/nodedb/src/config/server/env/rows/observability.rs b/nodedb/src/config/server/env/rows/observability.rs new file mode 100644 index 000000000..788596e35 --- /dev/null +++ b/nodedb/src/config/server/env/rows/observability.rs @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_PROMQL_ENABLED` / `NODEDB_OTLP_*` / `NODEDB_DEBUG_ENDPOINTS_ENABLED` +//! overrides. + +use crate::config::server::ServerConfig; + +use super::super::parse::{parse_bool_lenient, parse_socket_addr, parse_u64_positive}; +use super::super::table::EnvRow; +use crate::config::server::domain::otlp_endpoint_has_host; + +fn apply_promql_enabled(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.promql.enabled = parse_bool_lenient(raw)?; + Ok(()) +} + +fn apply_otlp_receiver_enabled(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.otlp.receiver.enabled = parse_bool_lenient(raw)?; + Ok(()) +} + +fn apply_otlp_http_listen(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.otlp.receiver.http_listen = + parse_socket_addr(raw, "a socket address such as 0.0.0.0:4318")?; + Ok(()) +} + +fn apply_otlp_grpc_listen(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.otlp.receiver.grpc_listen = + parse_socket_addr(raw, "a socket address such as 0.0.0.0:4317")?; + Ok(()) +} + +fn apply_otlp_export_enabled(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.otlp.export.enabled = parse_bool_lenient(raw)?; + Ok(()) +} + +/// Must carry a scheme and a non-empty host: `http://collector` is the +/// shortest value that can actually be dialed. +fn apply_otlp_export_endpoint(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + if !otlp_endpoint_has_host(raw) { + return Err("an http:// or https:// endpoint URL"); + } + config.observability.otlp.export.endpoint = raw.to_string(); + Ok(()) +} + +fn apply_otlp_export_interval(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.otlp.export.metrics_interval_secs = parse_u64_positive(raw)?; + Ok(()) +} + +fn apply_debug_endpoints_enabled(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.observability.debug_endpoints_enabled = parse_bool_lenient(raw)?; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_PROMQL_ENABLED", + apply: apply_promql_enabled, + redact: false, + }, + EnvRow { + name: "NODEDB_OTLP_RECEIVER_ENABLED", + apply: apply_otlp_receiver_enabled, + redact: false, + }, + EnvRow { + name: "NODEDB_OTLP_HTTP_LISTEN", + apply: apply_otlp_http_listen, + redact: false, + }, + EnvRow { + name: "NODEDB_OTLP_GRPC_LISTEN", + apply: apply_otlp_grpc_listen, + redact: false, + }, + EnvRow { + name: "NODEDB_OTLP_EXPORT_ENABLED", + apply: apply_otlp_export_enabled, + redact: false, + }, + EnvRow { + name: "NODEDB_OTLP_EXPORT_ENDPOINT", + apply: apply_otlp_export_endpoint, + redact: false, + }, + EnvRow { + name: "NODEDB_OTLP_EXPORT_INTERVAL", + apply: apply_otlp_export_interval, + redact: false, + }, + EnvRow { + name: "NODEDB_DEBUG_ENDPOINTS_ENABLED", + apply: apply_debug_endpoints_enabled, + redact: false, + }, +]; diff --git a/nodedb/src/config/server/env/rows/sizing.rs b/nodedb/src/config/server/env/rows/sizing.rs new file mode 100644 index 000000000..030221d13 --- /dev/null +++ b/nodedb/src/config/server/env/rows/sizing.rs @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_MEMORY_LIMIT` / `NODEDB_DATA_PLANE_CORES` / `NODEDB_MAX_CONNECTIONS` +//! / `NODEDB_LOG_FORMAT` overrides — process sizing and admission knobs. + +use crate::config::server::{LogFormat, ServerConfig}; + +use super::super::memory_size::parse_memory_size; +use super::super::parse::{parse_usize_nonneg, parse_usize_positive}; +use super::super::table::EnvRow; + +fn apply_memory_limit(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.memory_limit = + parse_memory_size(raw).map_err(|_| "a memory size such as 4GiB")?; + Ok(()) +} + +fn apply_data_plane_cores(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.data_plane_cores = parse_usize_positive(raw)?; + Ok(()) +} + +/// Zero is legal here and means unlimited, unlike every other sizing row. +fn apply_max_connections(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.max_connections = parse_usize_nonneg(raw)?; + Ok(()) +} + +fn apply_log_format(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.server.log_format = match raw.trim().to_ascii_lowercase().as_str() { + "text" => LogFormat::Text, + "json" => LogFormat::Json, + _ => return Err("\"text\" or \"json\""), + }; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_MEMORY_LIMIT", + apply: apply_memory_limit, + redact: false, + }, + EnvRow { + name: "NODEDB_DATA_PLANE_CORES", + apply: apply_data_plane_cores, + redact: false, + }, + EnvRow { + name: "NODEDB_MAX_CONNECTIONS", + apply: apply_max_connections, + redact: false, + }, + EnvRow { + name: "NODEDB_LOG_FORMAT", + apply: apply_log_format, + redact: false, + }, +]; + +#[cfg(test)] +mod tests { + use super::super::super::apply_env_overrides; + use super::*; + + #[test] + fn env_memory_limit_overrides() { + unsafe { std::env::set_var("NODEDB_MEMORY_LIMIT", "2GiB") }; + let mut cfg = ServerConfig::default(); + apply_env_overrides(&mut cfg).expect("a valid memory size must apply"); + assert_eq!(cfg.server.memory_limit, 2 * 1024 * 1024 * 1024); + unsafe { std::env::remove_var("NODEDB_MEMORY_LIMIT") }; + } +} diff --git a/nodedb/src/config/server/env/rows/timeseries.rs b/nodedb/src/config/server/env/rows/timeseries.rs new file mode 100644 index 000000000..e78f23410 --- /dev/null +++ b/nodedb/src/config/server/env/rows/timeseries.rs @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Timeseries memtable admission knobs: `NODEDB_TS_MEMTABLE_BUDGET_BYTES`, +//! `NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES`, `NODEDB_TS_MAX_TAG_CARDINALITY`. + +use crate::config::server::ServerConfig; + +use super::super::parse::{parse_u32_positive, parse_usize_positive}; +use super::super::table::EnvRow; + +fn apply_memtable_budget_bytes(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.tuning.timeseries.memtable_budget_bytes = parse_usize_positive(raw)?; + Ok(()) +} + +fn apply_memtable_hard_limit_bytes( + config: &mut ServerConfig, + raw: &str, +) -> Result<(), &'static str> { + config.tuning.timeseries.memtable_hard_limit_bytes = parse_usize_positive(raw)?; + Ok(()) +} + +fn apply_max_tag_cardinality(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.tuning.timeseries.max_tag_cardinality = parse_u32_positive(raw)?; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_TS_MEMTABLE_BUDGET_BYTES", + apply: apply_memtable_budget_bytes, + redact: false, + }, + EnvRow { + name: "NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES", + apply: apply_memtable_hard_limit_bytes, + redact: false, + }, + EnvRow { + name: "NODEDB_TS_MAX_TAG_CARDINALITY", + apply: apply_max_tag_cardinality, + redact: false, + }, +]; diff --git a/nodedb/src/config/server/env/rows/tls.rs b/nodedb/src/config/server/env/rows/tls.rs new file mode 100644 index 000000000..58b8de748 --- /dev/null +++ b/nodedb/src/config/server/env/rows/tls.rs @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_TLS_CERT_PATH` / `NODEDB_TLS_KEY_PATH` — which can create +//! `[server.tls]` — and the per-protocol `NODEDB_TLS_*` toggles, which +//! require it already exists. + +use std::path::PathBuf; + +use crate::config::server::{ServerConfig, TlsSettings}; + +use super::super::parse::parse_bool_lenient; +use super::super::table::EnvRow; + +/// Requires `NODEDB_TLS_KEY_PATH` also set — read directly rather than +/// through the dispatcher, since sibling rows are evaluated independently. +fn apply_cert_path(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + if std::env::var("NODEDB_TLS_KEY_PATH").is_err() { + return Err("NODEDB_TLS_KEY_PATH to be set alongside it"); + } + ensure_tls(config).cert_path = PathBuf::from(raw); + Ok(()) +} + +fn apply_key_path(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + if std::env::var("NODEDB_TLS_CERT_PATH").is_err() { + return Err("NODEDB_TLS_CERT_PATH to be set alongside it"); + } + ensure_tls(config).key_path = PathBuf::from(raw); + Ok(()) +} + +/// Returns `[server.tls]`, creating it with every protocol toggle on and no +/// cert-reload interval when none exists yet. +fn ensure_tls(config: &mut ServerConfig) -> &mut TlsSettings { + config.server.tls.get_or_insert_with(|| TlsSettings { + cert_path: PathBuf::new(), + key_path: PathBuf::new(), + cert_reload_interval_secs: None, + native: true, + pgwire: true, + http: true, + resp: true, + ilp: true, + }) +} + +/// With `[server.tls]` present, sets the field. With no section and a +/// `false` request, the listener is already plaintext, so the request is +/// already satisfied. With no section and a `true` request, TLS cannot be +/// honored: no cert material exists to serve it. +fn apply_toggle( + config: &mut ServerConfig, + raw: &str, + field: impl FnOnce(&mut TlsSettings) -> &mut bool, +) -> Result<(), &'static str> { + let requested = parse_bool_lenient(raw)?; + match config.server.tls.as_mut() { + Some(tls) => { + *field(tls) = requested; + Ok(()) + } + None if !requested => Ok(()), + None => Err("a [server.tls] section, or NODEDB_TLS_CERT_PATH and NODEDB_TLS_KEY_PATH"), + } +} + +fn apply_native(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + apply_toggle(config, raw, |tls| &mut tls.native) +} + +fn apply_pgwire(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + apply_toggle(config, raw, |tls| &mut tls.pgwire) +} + +fn apply_http(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + apply_toggle(config, raw, |tls| &mut tls.http) +} + +fn apply_resp(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + apply_toggle(config, raw, |tls| &mut tls.resp) +} + +fn apply_ilp(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + apply_toggle(config, raw, |tls| &mut tls.ilp) +} + +pub(in super::super) const CERT_KEY_ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_TLS_CERT_PATH", + apply: apply_cert_path, + redact: false, + }, + EnvRow { + name: "NODEDB_TLS_KEY_PATH", + apply: apply_key_path, + redact: false, + }, +]; + +pub(in super::super) const TOGGLE_ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_TLS_NATIVE", + apply: apply_native, + redact: false, + }, + EnvRow { + name: "NODEDB_TLS_PGWIRE", + apply: apply_pgwire, + redact: false, + }, + EnvRow { + name: "NODEDB_TLS_HTTP", + apply: apply_http, + redact: false, + }, + EnvRow { + name: "NODEDB_TLS_RESP", + apply: apply_resp, + redact: false, + }, + EnvRow { + name: "NODEDB_TLS_ILP", + apply: apply_ilp, + redact: false, + }, +]; diff --git a/nodedb/src/config/server/env/rows/wal.rs b/nodedb/src/config/server/env/rows/wal.rs new file mode 100644 index 000000000..394bbe8d8 --- /dev/null +++ b/nodedb/src/config/server/env/rows/wal.rs @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `NODEDB_WAL_DIRECT_IO` / `NODEDB_WAL_WRITE_BUFFER_SIZE` overrides. + +use crate::config::server::ServerConfig; + +use super::super::memory_size::parse_memory_size; +use super::super::parse::parse_bool_lenient; +use super::super::table::EnvRow; +use crate::config::server::domain::MIN_WAL_WRITE_BUFFER_BYTES; + +fn apply_direct_io(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + config.tuning.wal.direct_io = parse_bool_lenient(raw)?; + Ok(()) +} + +fn apply_write_buffer_size(config: &mut ServerConfig, raw: &str) -> Result<(), &'static str> { + let bytes = parse_memory_size(raw).map_err(|_| "a memory size of at least 64KiB")?; + if bytes < MIN_WAL_WRITE_BUFFER_BYTES { + return Err("a memory size of at least 64KiB"); + } + config.tuning.wal.write_buffer_size = bytes; + Ok(()) +} + +pub(in super::super) const ROWS: &[EnvRow] = &[ + EnvRow { + name: "NODEDB_WAL_DIRECT_IO", + apply: apply_direct_io, + redact: false, + }, + EnvRow { + name: "NODEDB_WAL_WRITE_BUFFER_SIZE", + apply: apply_write_buffer_size, + redact: false, + }, +]; + +#[cfg(test)] +mod tests { + use super::super::super::apply_env_overrides; + use super::*; + + /// Direct I/O is the shipped default. Only an explicit opt-out turns it + /// off. + #[test] + fn env_wal_direct_io_override() { + unsafe { std::env::set_var("NODEDB_WAL_DIRECT_IO", "false") }; + let mut cfg = ServerConfig::default(); + apply_env_overrides(&mut cfg).expect("a valid direct-io toggle must apply"); + assert!(!cfg.tuning.wal.direct_io); + unsafe { std::env::remove_var("NODEDB_WAL_DIRECT_IO") }; + } +} diff --git a/nodedb/src/config/server/env/seed_nodes.rs b/nodedb/src/config/server/env/seed_nodes.rs new file mode 100644 index 000000000..6b4b1c020 --- /dev/null +++ b/nodedb/src/config/server/env/seed_nodes.rs @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Parses `NODEDB_SEED_NODES`: a comma-separated list of `host:port` +//! addresses. + +use std::net::SocketAddr; + +/// Parses a comma-separated list of `SocketAddr` entries. +/// +/// The `Err` text names the expected shape, not the offending entry. The +/// caller's own violation message already carries the full raw value, so +/// the bad entry survives there as a substring. +pub fn parse_seed_nodes(raw: &str) -> Result, &'static str> { + let mut addrs = Vec::new(); + for entry in raw.split(',') { + let entry = entry.trim(); + if entry.is_empty() { + continue; + } + let addr = entry + .parse::() + .map_err(|_| "a comma-separated list of host:port addresses")?; + addrs.push(addr); + } + Ok(addrs) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_multiple_addresses() { + let addrs = parse_seed_nodes("10.0.0.1:9400,10.0.0.2:9400").expect("valid addresses"); + assert_eq!(addrs.len(), 2); + assert_eq!(addrs[0].to_string(), "10.0.0.1:9400"); + assert_eq!(addrs[1].to_string(), "10.0.0.2:9400"); + } + + #[test] + fn rejects_bad_entry() { + assert!(parse_seed_nodes("10.0.0.1:9400,garbage").is_err()); + } +} diff --git a/nodedb/src/config/server/env/table.rs b/nodedb/src/config/server/env/table.rs new file mode 100644 index 000000000..23bc75400 --- /dev/null +++ b/nodedb/src/config/server/env/table.rs @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! The environment-override startup gate. +//! +//! One row per `NODEDB_*` variable, walked in order. The gate applies an +//! operator-supplied value, or refuses to start. It never substitutes a config +//! value or a compiled default. The gate collects every violation before +//! returning, so one bad value never hides the next. + +use super::rows::{ + checkpoint, cluster, host_ports, maintenance, observability, sizing, timeseries, tls, wal, +}; +use crate::config::server::ServerConfig; + +/// One `NODEDB_*` override. +pub(super) struct EnvRow { + /// `NODEDB_*` name, matched verbatim. + pub name: &'static str, + /// Applies the raw value. `Err` names what the process needed. + pub apply: fn(&mut ServerConfig, &str) -> Result<(), &'static str>, + /// `true` logs the name and never the value. + pub redact: bool, +} + +/// Row groups in startup-gate order. +/// +/// `NODEDB_TLS_CERT_PATH` and `NODEDB_TLS_KEY_PATH` run first: they can +/// create `[server.tls]`, which the TLS toggle rows further down require. +const TABLE: &[&[EnvRow]] = &[ + tls::CERT_KEY_ROWS, + host_ports::ROWS, + sizing::ROWS, + tls::TOGGLE_ROWS, + cluster::ROWS, + wal::ROWS, + checkpoint::ROWS, + timeseries::ROWS, + maintenance::ROWS, + observability::ROWS, +]; + +/// Applies every `NODEDB_*` override present in the environment to `config`. +/// +/// Returns every violation joined into one [`crate::Error::Config`], or +/// `Ok(())` once none remain. A run with any violation must exit the +/// process, so partial application here never reaches a running server. +pub fn apply_env_overrides(config: &mut ServerConfig) -> crate::Result<()> { + let mut violations = Vec::new(); + + for group in TABLE { + for row in *group { + let Ok(raw) = std::env::var(row.name) else { + continue; + }; + if raw.trim().is_empty() { + violations.push(format!( + "invalid value '{raw}' for {}: expected a non-empty value", + row.name + )); + continue; + } + match (row.apply)(config, &raw) { + Ok(()) => { + let logged: &str = if row.redact { + "" + } else { + raw.as_str() + }; + tracing::info!( + env_var = row.name, + value = logged, + "environment variable override applied" + ); + } + Err(expected) => { + violations.push(format!( + "invalid value '{raw}' for {}: expected {expected}", + row.name + )); + } + } + } + } + + if violations.is_empty() { + Ok(()) + } else { + Err(crate::Error::Config { + detail: violations.join("; "), + }) + } +} diff --git a/nodedb/src/config/server/env/timeseries.rs b/nodedb/src/config/server/env/timeseries.rs deleted file mode 100644 index b7c4efdce..000000000 --- a/nodedb/src/config/server/env/timeseries.rs +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! Timeseries memtable admission knobs: `NODEDB_TS_MEMTABLE_BUDGET_BYTES`, -//! `NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES`, `NODEDB_TS_MAX_TAG_CARDINALITY`. -//! -//! These are env-reachable because the record-boundary admission gate they -//! drive is otherwise only observable at 64/80 MiB and 100k distinct tags — -//! a cost no test can pay, which is how a mid-record flush stamping a -//! partition with the WRONG WAL LSN went unnoticed. Defaults are unchanged; -//! this only makes them reachable. - -use super::helpers::apply_positive_env; -use crate::config::server::ServerConfig; - -pub(super) fn apply_timeseries_overrides(config: &mut ServerConfig) { - apply_positive_env( - "NODEDB_TS_MEMTABLE_BUDGET_BYTES", - &mut config.tuning.timeseries.memtable_budget_bytes, - ); - apply_positive_env( - "NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES", - &mut config.tuning.timeseries.memtable_hard_limit_bytes, - ); - apply_positive_env( - "NODEDB_TS_MAX_TAG_CARDINALITY", - &mut config.tuning.timeseries.max_tag_cardinality, - ); -} diff --git a/nodedb/src/config/server/env/tls.rs b/nodedb/src/config/server/env/tls.rs deleted file mode 100644 index 389ce3f3d..000000000 --- a/nodedb/src/config/server/env/tls.rs +++ /dev/null @@ -1,17 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! Per-protocol `NODEDB_TLS_*` toggles. No-op if the config has no `[tls]` -//! section at all — TLS is enabled/disabled per protocol, not created here. - -use super::helpers::apply_bool_env; -use crate::config::server::ServerConfig; - -pub(super) fn apply_tls_overrides(config: &mut ServerConfig) { - if let Some(ref mut tls) = config.server.tls { - apply_bool_env("NODEDB_TLS_NATIVE", &mut tls.native); - apply_bool_env("NODEDB_TLS_PGWIRE", &mut tls.pgwire); - apply_bool_env("NODEDB_TLS_HTTP", &mut tls.http); - apply_bool_env("NODEDB_TLS_RESP", &mut tls.resp); - apply_bool_env("NODEDB_TLS_ILP", &mut tls.ilp); - } -} diff --git a/nodedb/src/config/server/env/wal.rs b/nodedb/src/config/server/env/wal.rs deleted file mode 100644 index fc3d494c7..000000000 --- a/nodedb/src/config/server/env/wal.rs +++ /dev/null @@ -1,72 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 - -//! `NODEDB_WAL_DIRECT_IO` / `NODEDB_WAL_WRITE_BUFFER_SIZE` overrides. - -use super::helpers::apply_bool_env; -use super::memory_size::parse_memory_size; -use crate::config::server::ServerConfig; - -pub(super) fn apply_wal_tuning(config: &mut ServerConfig) { - // On by default. Env-reachable because the one case that legitimately - // needs it off — a data directory on a filesystem without direct-I/O - // support, such as a harness tempdir on tmpfs — is a property of the - // deployment, not of the config file shipped with it. - apply_bool_env("NODEDB_WAL_DIRECT_IO", &mut config.tuning.wal.direct_io); - - if let Ok(val) = std::env::var("NODEDB_WAL_WRITE_BUFFER_SIZE") { - match parse_memory_size(&val) { - Ok(size) if size >= 64 * 1024 => { - tracing::info!( - env_var = "NODEDB_WAL_WRITE_BUFFER_SIZE", - value = size, - "environment variable override applied" - ); - config.tuning.wal.write_buffer_size = size; - } - Ok(size) => { - tracing::warn!( - env_var = "NODEDB_WAL_WRITE_BUFFER_SIZE", - value = size, - "ignoring value below minimum 64KiB, using config value" - ); - } - Err(_) => { - tracing::warn!( - env_var = "NODEDB_WAL_WRITE_BUFFER_SIZE", - value = %val, - "ignoring malformed environment variable, using config value" - ); - } - } - } -} - -#[cfg(test)] -mod tests { - use super::super::dispatch::apply_env_overrides; - use super::*; - - /// Direct I/O is the shipped default, and only an explicit opt-out turns - /// it off — an absent or malformed env var must never be read as one. - #[test] - fn env_wal_direct_io_override() { - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert!(cfg.tuning.wal.direct_io, "default must be direct I/O"); - - unsafe { std::env::set_var("NODEDB_WAL_DIRECT_IO", "false") }; - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert!(!cfg.tuning.wal.direct_io); - - unsafe { std::env::set_var("NODEDB_WAL_DIRECT_IO", "nonsense") }; - let mut cfg = ServerConfig::default(); - apply_env_overrides(&mut cfg); - assert!( - cfg.tuning.wal.direct_io, - "a malformed value must not silently disable direct I/O" - ); - - unsafe { std::env::remove_var("NODEDB_WAL_DIRECT_IO") }; - } -} diff --git a/nodedb/src/config/server/mod.rs b/nodedb/src/config/server/mod.rs index a049fe8c3..ccd331b12 100644 --- a/nodedb/src/config/server/mod.rs +++ b/nodedb/src/config/server/mod.rs @@ -4,6 +4,7 @@ mod checkpoint; mod cluster; mod cold_storage; mod config; +mod domain; mod env; mod log_format; mod observability; @@ -23,7 +24,7 @@ pub use env::{apply_env_overrides, parse_memory_size, parse_seed_nodes}; pub use log_format::LogFormat; pub use observability::{ ObservabilityConfig, OtlpConfig, OtlpExportConfig, OtlpReceiverConfig, PromqlConfig, - apply_observability_env, validate_feature_availability, + validate_feature_availability, }; pub use ports::{DEFAULT_SYNC_PORT, PortsConfig}; pub use retention::RetentionSettings; diff --git a/nodedb/src/config/server/observability.rs b/nodedb/src/config/server/observability.rs index ccd96f788..4f9847448 100644 --- a/nodedb/src/config/server/observability.rs +++ b/nodedb/src/config/server/observability.rs @@ -146,92 +146,6 @@ fn default_metrics_interval() -> u64 { 15 } -/// Apply observability-related environment variable overrides. -/// -/// Variables: -/// - `NODEDB_PROMQL_ENABLED` — "true"/"false" -/// - `NODEDB_OTLP_RECEIVER_ENABLED` — "true"/"false" -/// - `NODEDB_OTLP_HTTP_LISTEN` — SocketAddr -/// - `NODEDB_OTLP_GRPC_LISTEN` — SocketAddr -/// - `NODEDB_OTLP_EXPORT_ENABLED` — "true"/"false" -/// - `NODEDB_OTLP_EXPORT_ENDPOINT` — URL string -/// - `NODEDB_OTLP_EXPORT_INTERVAL` — seconds (u64) -pub fn apply_observability_env(config: &mut ObservabilityConfig) { - if let Ok(val) = std::env::var("NODEDB_PROMQL_ENABLED") - && let Ok(b) = val.parse::() - { - tracing::info!( - env_var = "NODEDB_PROMQL_ENABLED", - value = b, - "override applied" - ); - config.promql.enabled = b; - } - - if let Ok(val) = std::env::var("NODEDB_OTLP_RECEIVER_ENABLED") - && let Ok(b) = val.parse::() - { - tracing::info!( - env_var = "NODEDB_OTLP_RECEIVER_ENABLED", - value = b, - "override applied" - ); - config.otlp.receiver.enabled = b; - } - - if let Ok(val) = std::env::var("NODEDB_OTLP_HTTP_LISTEN") - && let Ok(addr) = val.parse::() - { - tracing::info!(env_var = "NODEDB_OTLP_HTTP_LISTEN", value = %val, "override applied"); - config.otlp.receiver.http_listen = addr; - } - - if let Ok(val) = std::env::var("NODEDB_OTLP_GRPC_LISTEN") - && let Ok(addr) = val.parse::() - { - tracing::info!(env_var = "NODEDB_OTLP_GRPC_LISTEN", value = %val, "override applied"); - config.otlp.receiver.grpc_listen = addr; - } - - if let Ok(val) = std::env::var("NODEDB_OTLP_EXPORT_ENABLED") - && let Ok(b) = val.parse::() - { - tracing::info!( - env_var = "NODEDB_OTLP_EXPORT_ENABLED", - value = b, - "override applied" - ); - config.otlp.export.enabled = b; - } - - if let Ok(val) = std::env::var("NODEDB_OTLP_EXPORT_ENDPOINT") { - tracing::info!(env_var = "NODEDB_OTLP_EXPORT_ENDPOINT", value = %val, "override applied"); - config.otlp.export.endpoint = val; - } - - if let Ok(val) = std::env::var("NODEDB_OTLP_EXPORT_INTERVAL") - && let Ok(secs) = val.parse::() - { - tracing::info!( - env_var = "NODEDB_OTLP_EXPORT_INTERVAL", - value = secs, - "override applied" - ); - config.otlp.export.metrics_interval_secs = secs; - } - - if let Ok(val) = std::env::var("NODEDB_DEBUG_ENDPOINTS_ENABLED") - && let Ok(b) = val.parse::() - { - tracing::info!( - env_var = "NODEDB_DEBUG_ENDPOINTS_ENABLED", - value = b, - "override applied" - ); - config.debug_endpoints_enabled = b; - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/nodedb/src/control/cluster/init.rs b/nodedb/src/control/cluster/init.rs index 178696a08..059eb608f 100644 --- a/nodedb/src/control/cluster/init.rs +++ b/nodedb/src/control/cluster/init.rs @@ -93,7 +93,10 @@ pub async fn init_cluster_with_transport( replication_factor: config.replication_factor, data_dir: data_dir.to_path_buf(), force_bootstrap: config.force_bootstrap, - join_retry: join_retry_policy_from_env(), + join_retry: nodedb_cluster::JoinRetryPolicy { + max_attempts: config.join_retry_max_attempts, + max_backoff_secs: config.join_retry_max_backoff_secs, + }, swim_udp_addr: None, election_timeout_min: std::time::Duration::from_millis( transport_tuning.effective_election_timeout_min_ms(), @@ -224,34 +227,9 @@ pub async fn init_single_node_calvin( login_attempts_per_user_per_min: 0, insecure_transport: true, log_compaction_threshold: None, + join_retry_max_attempts: 8, + join_retry_max_backoff_secs: 32, }; init_cluster_with_transport(&settings, transport, data_dir, transport_tuning).await } - -/// Build the join retry policy, honouring two optional environment -/// variables for test/CI overrides: -/// -/// - `NODEDB_JOIN_RETRY_MAX_ATTEMPTS` — total attempts (default 8) -/// - `NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS` — per-attempt ceiling -/// (default 32 s) -/// -/// Production deployments leave both unset and get the production -/// schedule. The integration test harness sets both to small values -/// so a join-retry path doesn't spend ~1 minute sleeping in CI. -fn join_retry_policy_from_env() -> nodedb_cluster::JoinRetryPolicy { - let mut policy = nodedb_cluster::JoinRetryPolicy::default(); - if let Ok(v) = std::env::var("NODEDB_JOIN_RETRY_MAX_ATTEMPTS") - && let Ok(n) = v.parse::() - && n > 0 - { - policy.max_attempts = n; - } - if let Ok(v) = std::env::var("NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS") - && let Ok(n) = v.parse::() - && n > 0 - { - policy.max_backoff_secs = n; - } - policy -} diff --git a/nodedb/src/control/cluster/tls.rs b/nodedb/src/control/cluster/tls.rs index cc0440130..e981cedad 100644 --- a/nodedb/src/control/cluster/tls.rs +++ b/nodedb/src/control/cluster/tls.rs @@ -526,6 +526,8 @@ mod tests { login_attempts_per_user_per_min: 10, insecure_transport: false, log_compaction_threshold: None, + join_retry_max_attempts: 8, + join_retry_max_backoff_secs: 32, } } diff --git a/nodedb/src/control/security/scope/expiry.rs b/nodedb/src/control/security/scope/expiry.rs index 0039241ee..f59cb1bd6 100644 --- a/nodedb/src/control/security/scope/expiry.rs +++ b/nodedb/src/control/security/scope/expiry.rs @@ -26,15 +26,12 @@ use super::grant::{ScopeGrantParams, ScopeStatus}; /// /// The pass itself is synchronous and writes redb, so it runs on a blocking /// thread rather than the reactor. -pub fn spawn_expiry_task(shared: Arc) { - let interval_secs = std::env::var("NODEDB_SCOPE_EXPIRY_INTERVAL_SECS") - .ok() - .and_then(|v| v.parse::().ok()) - .unwrap_or(60); +pub fn spawn_expiry_task(shared: Arc, interval_secs: u64) { // Below ~10s the sweep costs more than the resolution it buys: expiry is // already enforced on every read by `ScopeGrant::is_effective`, and this - // loop only makes the outcome durable. - let interval = Duration::from_secs(interval_secs.max(10)); + // loop only makes the outcome durable. The startup config gate rejects + // an interval below that floor, so no clamp runs here. + let interval = Duration::from_secs(interval_secs); info!(interval_secs, "scope expiry sweep loop running"); let loop_shared = Arc::clone(&shared); crate::control::shutdown::spawn_loop( diff --git a/nodedb/src/main.rs b/nodedb/src/main.rs index d22d3ad15..b7dd17820 100644 --- a/nodedb/src/main.rs +++ b/nodedb/src/main.rs @@ -93,10 +93,12 @@ async fn server_main() -> anyhow::Result<()> { }; // Apply env overrides once now (before tracing) so that log_format is - // correct in case NODEDB_DATA_DIR / NODEDB_MEMORY_LIMIT also affect it. - // The overrides are re-applied silently here; the real log messages - // will be emitted by the second call after the subscriber is registered. - apply_env_overrides(&mut config); + // correct in case NODEDB_DATA_DIR / NODEDB_MEMORY_LIMIT also affect it, + // and so a malformed override aborts the boot before any subsystem + // starts. The info! messages this call emits reach no subscriber yet; + // the second call below re-applies them once tracing is registered so + // the operator still sees them logged. + apply_env_overrides(&mut config)?; // Own the black-box recorder before the subscriber is built: the panic hook // it installs chains in front of the one above, and the reports directory @@ -126,7 +128,7 @@ async fn server_main() -> anyhow::Result<()> { // Re-apply env overrides after tracing initializes so that // info!/warn! messages are actually emitted for operators. - apply_env_overrides(&mut config); + apply_env_overrides(&mut config)?; let cluster_mode_str = startup_log::log_boot_banner(&config_path, &config); diff --git a/nodedb/tests/config_env_cluster.rs b/nodedb/tests/config_env_cluster.rs new file mode 100644 index 000000000..73a06627e --- /dev/null +++ b/nodedb/tests/config_env_cluster.rs @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Startup rejection of malformed and inapplicable cluster overrides. +//! +//! Orchestrated deployments set cluster identity and membership from the +//! environment. A node that boots with a fallback identity joins the wrong +//! group, or fails to join. The operator learns it from the cluster. + +mod support; + +use nodedb::ServerConfig; +use nodedb::config::server::{ClusterSettings, apply_env_overrides}; +use support::env_guard::{EnvGuard, assert_rejected}; + +fn cluster_config(node_id: u64) -> ServerConfig { + ServerConfig { + cluster: Some(ClusterSettings { + node_id, + listen: "0.0.0.0:9400".parse().expect("listen address"), + seed_nodes: vec!["127.0.0.1:9400".parse().expect("seed address")], + num_groups: 4, + replication_factor: 3, + force_bootstrap: false, + tls: None, + max_active_sessions: 0, + login_attempts_per_ip_per_min: 30, + login_attempts_per_user_per_min: 10, + insecure_transport: false, + log_compaction_threshold: None, + join_retry_max_attempts: 8, + join_retry_max_backoff_secs: 32, + }), + ..Default::default() + } +} + +#[test] +fn malformed_node_id_fails_startup() { + let _guard = EnvGuard::set("NODEDB_NODE_ID", "not_a_number"); + let mut cfg = cluster_config(7); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_NODE_ID", + "not_a_number", + ); +} + +/// A node id set with no `[cluster]` section is well-formed and still lost. +/// The operator asked for a cluster member. A standalone node that answers +/// queries hides the mistake better than a refused boot. +#[test] +fn node_id_without_cluster_section_fails_startup() { + let _guard = EnvGuard::set("NODEDB_NODE_ID", "42"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_NODE_ID", "42"); +} + +#[test] +fn malformed_seed_node_entry_fails_startup() { + let _guard = EnvGuard::set("NODEDB_SEED_NODES", "10.0.0.1:9400,garbage"); + let mut cfg = cluster_config(1); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_SEED_NODES", + "garbage", + ); +} + +/// One bad entry must not drop the whole list back to the config value. The +/// surviving seeds are the ones the operator did not choose. +#[test] +fn seed_node_missing_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_SEED_NODES", "10.0.0.1,10.0.0.2:9400"); + let mut cfg = cluster_config(1); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_SEED_NODES", + "10.0.0.1", + ); +} + +#[test] +fn seed_nodes_without_cluster_section_fails_startup() { + let _guard = EnvGuard::set("NODEDB_SEED_NODES", "10.0.0.1:9400"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_SEED_NODES", + "10.0.0.1:9400", + ); +} + +/// Cluster bring-up reads the join retry policy far from config load. +/// Validating it there is too late to refuse the boot. The gate checks it in +/// the same pass as every other override. +#[test] +fn malformed_join_retry_max_attempts_fails_startup() { + let _guard = EnvGuard::set("NODEDB_JOIN_RETRY_MAX_ATTEMPTS", "lots"); + let mut cfg = cluster_config(1); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_JOIN_RETRY_MAX_ATTEMPTS", + "lots", + ); +} + +#[test] +fn zero_join_retry_max_attempts_fails_startup() { + let _guard = EnvGuard::set("NODEDB_JOIN_RETRY_MAX_ATTEMPTS", "0"); + let mut cfg = cluster_config(1); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_JOIN_RETRY_MAX_ATTEMPTS", + "0", + ); +} + +#[test] +fn malformed_join_retry_max_backoff_fails_startup() { + let _guard = EnvGuard::set("NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS", "30s"); + let mut cfg = cluster_config(1); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS", + "30s", + ); +} diff --git a/nodedb/tests/config_env_durability.rs b/nodedb/tests/config_env_durability.rs new file mode 100644 index 000000000..75484a31d --- /dev/null +++ b/nodedb/tests/config_env_durability.rs @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Startup rejection of malformed WAL and checkpoint overrides. +//! +//! These variables decide write durability and WAL trimming. A fallback here +//! costs the most in this class. The boot looks configured. The difference +//! surfaces as a lost write after a crash, or as an unbounded WAL. + +mod support; + +use nodedb::ServerConfig; +use nodedb::config::server::apply_env_overrides; +use support::env_guard::{EnvGuard, assert_rejected}; + +/// Direct I/O ships on, and only an explicit opt-out turns it off. A +/// malformed value is neither an opt-out nor an opt-in. It is an operator who +/// believes they set the flag and did not. +#[test] +fn malformed_wal_direct_io_fails_startup() { + let _guard = EnvGuard::set("NODEDB_WAL_DIRECT_IO", "nonsense"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_WAL_DIRECT_IO", + "nonsense", + ); +} + +#[test] +fn malformed_wal_write_buffer_size_fails_startup() { + let _guard = EnvGuard::set("NODEDB_WAL_WRITE_BUFFER_SIZE", "1MB!"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_WAL_WRITE_BUFFER_SIZE", + "1MB!", + ); +} + +/// A buffer under the 64 KiB floor is a value the process refuses to honour. +/// Refusing it while starting anyway hands the operator the throughput profile +/// of the default, attributed to the size they set. +#[test] +fn below_minimum_wal_write_buffer_size_fails_startup() { + let _guard = EnvGuard::set("NODEDB_WAL_WRITE_BUFFER_SIZE", "4KiB"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_WAL_WRITE_BUFFER_SIZE", + "4KiB", + ); +} + +#[test] +fn malformed_checkpoint_interval_fails_startup() { + let _guard = EnvGuard::set("NODEDB_CHECKPOINT_INTERVAL_SECS", "5m"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_CHECKPOINT_INTERVAL_SECS", + "5m", + ); +} + +#[test] +fn zero_checkpoint_interval_fails_startup() { + let _guard = EnvGuard::set("NODEDB_CHECKPOINT_INTERVAL_SECS", "0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_CHECKPOINT_INTERVAL_SECS", + "0", + ); +} + +#[test] +fn malformed_wal_segment_target_fails_startup() { + let _guard = EnvGuard::set("NODEDB_WAL_SEGMENT_TARGET_MB", "64MiB"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_WAL_SEGMENT_TARGET_MB", + "64MiB", + ); +} + +#[test] +fn zero_wal_segment_target_fails_startup() { + let _guard = EnvGuard::set("NODEDB_WAL_SEGMENT_TARGET_MB", "0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_WAL_SEGMENT_TARGET_MB", + "0", + ); +} diff --git a/nodedb/tests/config_env_listeners.rs b/nodedb/tests/config_env_listeners.rs new file mode 100644 index 000000000..e25b55cac --- /dev/null +++ b/nodedb/tests/config_env_listeners.rs @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Startup rejection of malformed listener and memory overrides. +//! +//! A value that fails to parse stops the boot. The error names the variable +//! and the value. An orchestration typo therefore cannot move the node to +//! another address or port, or leave a listener plaintext. + +mod support; + +use nodedb::ServerConfig; +use nodedb::config::server::{TlsSettings, apply_env_overrides}; +use support::env_guard::{EnvGuard, assert_rejected}; + +fn with_tls() -> ServerConfig { + let mut cfg = ServerConfig::default(); + cfg.server.tls = Some(TlsSettings { + cert_path: std::path::PathBuf::from("/etc/nodedb/tls/server.crt"), + key_path: std::path::PathBuf::from("/etc/nodedb/tls/server.key"), + cert_reload_interval_secs: None, + native: true, + pgwire: true, + http: true, + resp: true, + ilp: true, + }); + cfg +} + +#[test] +fn malformed_host_fails_startup() { + let _guard = EnvGuard::set("NODEDB_HOST", "not-an-ip"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_HOST", "not-an-ip"); +} + +#[test] +fn malformed_sync_host_fails_startup() { + let _guard = EnvGuard::set("NODEDB_SYNC_HOST", "127.0.0.256"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_SYNC_HOST", + "127.0.0.256", + ); +} + +#[test] +fn malformed_native_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PORT_NATIVE", "sixty-four-thirty-three"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_PORT_NATIVE", + "sixty-four-thirty-three", + ); +} + +#[test] +fn out_of_range_pgwire_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PORT_PGWIRE", "70000"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_PORT_PGWIRE", "70000"); +} + +#[test] +fn malformed_http_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PORT_HTTP", "6480x"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_PORT_HTTP", "6480x"); +} + +#[test] +fn malformed_sync_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PORT_SYNC", "notaport"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_PORT_SYNC", + "notaport", + ); +} + +/// A malformed value on a listener that is off by default is not "leave it +/// off". The operator asked for RESP. Without this, the client fails to +/// connect instead of the server failing to start. +#[test] +fn malformed_resp_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PORT_RESP", "resp"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_PORT_RESP", "resp"); +} + +#[test] +fn malformed_ilp_port_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PORT_ILP", "8086;"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_PORT_ILP", "8086;"); +} + +#[test] +fn malformed_memory_limit_fails_startup() { + let _guard = EnvGuard::set("NODEDB_MEMORY_LIMIT", "4ZiB"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_MEMORY_LIMIT", "4ZiB"); +} + +#[test] +fn non_numeric_memory_limit_fails_startup() { + let _guard = EnvGuard::set("NODEDB_MEMORY_LIMIT", "notanumber"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_MEMORY_LIMIT", + "notanumber", + ); +} + +/// A malformed TLS toggle is the worst case in this class. A fallback keeps +/// the listener wherever the config file left it. The operator then gets the +/// opposite transport security from the one they asked for. +#[test] +fn malformed_native_tls_toggle_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_NATIVE", "yes-please"); + let mut cfg = with_tls(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TLS_NATIVE", + "yes-please", + ); +} + +#[test] +fn malformed_pgwire_tls_toggle_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_PGWIRE", "off"); + let mut cfg = with_tls(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_TLS_PGWIRE", "off"); +} + +#[test] +fn malformed_http_tls_toggle_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_HTTP", "disable"); + let mut cfg = with_tls(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_TLS_HTTP", "disable"); +} + +#[test] +fn malformed_resp_tls_toggle_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_RESP", "2"); + let mut cfg = with_tls(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_TLS_RESP", "2"); +} + +#[test] +fn malformed_ilp_tls_toggle_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_ILP", "nope"); + let mut cfg = with_tls(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_TLS_ILP", "nope"); +} + +/// Enabling TLS with no `[server.tls]` section is unsatisfiable. The process +/// holds no certificate material to serve. The boot refuses instead of +/// pretending the request was honored. +#[test] +fn tls_enable_without_tls_section_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_PGWIRE", "true"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_TLS_PGWIRE", "true"); +} + +/// Both cert and key paths set with no prior `[server.tls]` section create +/// one, with every protocol toggle on by default. +#[test] +fn tls_cert_and_key_paths_create_tls_section() { + let _guard = EnvGuard::set_all(&[ + ("NODEDB_TLS_CERT_PATH", "/etc/nodedb/tls/server.crt"), + ("NODEDB_TLS_KEY_PATH", "/etc/nodedb/tls/server.key"), + ]); + let mut cfg = ServerConfig::default(); + apply_env_overrides(&mut cfg).expect("both paths set must create the tls section"); + let tls = cfg.server.tls.expect("tls section must be created"); + assert_eq!( + tls.cert_path, + std::path::PathBuf::from("/etc/nodedb/tls/server.crt") + ); + assert_eq!( + tls.key_path, + std::path::PathBuf::from("/etc/nodedb/tls/server.key") + ); + assert!(tls.native); + assert!(tls.pgwire); + assert!(tls.http); + assert!(tls.resp); + assert!(tls.ilp); +} + +/// A cert path with no key path is unsatisfiable: half a credential pair +/// cannot serve TLS. +#[test] +fn tls_cert_path_without_key_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_CERT_PATH", "/etc/nodedb/tls/server.crt"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TLS_CERT_PATH", + "/etc/nodedb/tls/server.crt", + ); +} + +/// A key path with no cert path is the symmetric half-a-credential case. +#[test] +fn tls_key_path_without_cert_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TLS_KEY_PATH", "/etc/nodedb/tls/server.key"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TLS_KEY_PATH", + "/etc/nodedb/tls/server.key", + ); +} + +/// A set-but-empty `NODEDB_DATA_DIR` is a failed template substitution, not +/// an operator choice of the current directory. +#[test] +fn empty_data_dir_fails_startup() { + let _guard = EnvGuard::set("NODEDB_DATA_DIR", ""); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_DATA_DIR", ""); +} diff --git a/nodedb/tests/config_env_maintenance.rs b/nodedb/tests/config_env_maintenance.rs new file mode 100644 index 000000000..6a440fb82 --- /dev/null +++ b/nodedb/tests/config_env_maintenance.rs @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Startup rejection of malformed background-loop interval overrides. +//! +//! The gate reads these in the same pass as every other override. A value read +//! at loop-spawn time cannot refuse a boot, because the process is already up. + +mod support; + +use nodedb::ServerConfig; +use nodedb::config::server::apply_env_overrides; +use support::env_guard::{EnvGuard, assert_rejected}; + +#[test] +fn malformed_clone_sweep_interval_fails_startup() { + let _guard = EnvGuard::set("NODEDB_CLONE_SWEEP_INTERVAL_MS", "30s"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_CLONE_SWEEP_INTERVAL_MS", + "30s", + ); +} + +#[test] +fn malformed_constraint_reconcile_interval_fails_startup() { + let _guard = EnvGuard::set("NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS", "1_000"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS", + "1_000", + ); +} + +#[test] +fn malformed_scope_expiry_interval_fails_startup() { + let _guard = EnvGuard::set("NODEDB_SCOPE_EXPIRY_INTERVAL_SECS", "sixty"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_SCOPE_EXPIRY_INTERVAL_SECS", + "sixty", + ); +} + +/// Below the 10-second floor the sweep costs more than the resolution it +/// buys. The value is out of domain, not merely small. +#[test] +fn scope_expiry_interval_below_floor_fails_startup() { + let _guard = EnvGuard::set("NODEDB_SCOPE_EXPIRY_INTERVAL_SECS", "5"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_SCOPE_EXPIRY_INTERVAL_SECS", + "5", + ); +} diff --git a/nodedb/tests/config_env_observability.rs b/nodedb/tests/config_env_observability.rs new file mode 100644 index 000000000..b9d7bba73 --- /dev/null +++ b/nodedb/tests/config_env_observability.rs @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Startup rejection of malformed observability overrides. +//! +//! A mistyped value here costs the metrics endpoint, the OTLP export, or the +//! debug-endpoint gate. The gate names the variable, so the boot log points +//! at the typo. + +mod support; + +use nodedb::ServerConfig; +use nodedb::config::server::apply_env_overrides; +use support::env_guard::{EnvGuard, assert_rejected}; + +#[test] +fn malformed_promql_enabled_fails_startup() { + let _guard = EnvGuard::set("NODEDB_PROMQL_ENABLED", "enabled"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_PROMQL_ENABLED", + "enabled", + ); +} + +#[test] +fn malformed_otlp_receiver_enabled_fails_startup() { + let _guard = EnvGuard::set("NODEDB_OTLP_RECEIVER_ENABLED", "sometimes"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_OTLP_RECEIVER_ENABLED", + "sometimes", + ); +} + +/// Every toggle in the override surface takes the same vocabulary. `1` on one +/// listener and `yes` on another get the same answer, here too. +#[test] +fn observability_toggles_take_the_shared_bool_vocabulary() { + let _guard = EnvGuard::set_all(&[ + ("NODEDB_PROMQL_ENABLED", "0"), + ("NODEDB_OTLP_RECEIVER_ENABLED", "1"), + ("NODEDB_OTLP_EXPORT_ENABLED", "yes"), + ("NODEDB_DEBUG_ENDPOINTS_ENABLED", "no"), + ]); + let mut cfg = ServerConfig::default(); + apply_env_overrides(&mut cfg).expect("shared bool vocabulary applies"); + assert!(!cfg.observability.promql.enabled); + assert!(cfg.observability.otlp.receiver.enabled); + assert!(cfg.observability.otlp.export.enabled); + assert!(!cfg.observability.debug_endpoints_enabled); +} + +#[test] +fn malformed_otlp_http_listen_fails_startup() { + let _guard = EnvGuard::set("NODEDB_OTLP_HTTP_LISTEN", "0.0.0.0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_OTLP_HTTP_LISTEN", + "0.0.0.0", + ); +} + +#[test] +fn malformed_otlp_grpc_listen_fails_startup() { + let _guard = EnvGuard::set("NODEDB_OTLP_GRPC_LISTEN", "localhost:4317"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_OTLP_GRPC_LISTEN", + "localhost:4317", + ); +} + +#[test] +fn malformed_otlp_export_enabled_fails_startup() { + let _guard = EnvGuard::set("NODEDB_OTLP_EXPORT_ENABLED", "on"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_OTLP_EXPORT_ENABLED", + "on", + ); +} + +#[test] +fn malformed_otlp_export_interval_fails_startup() { + let _guard = EnvGuard::set("NODEDB_OTLP_EXPORT_INTERVAL", "15s"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_OTLP_EXPORT_INTERVAL", + "15s", + ); +} + +/// The debug endpoints expose raft internals and the metadata cache. A +/// mistyped value on their gate stops the boot. It never resolves to whatever +/// the config file said. +#[test] +fn malformed_debug_endpoints_enabled_fails_startup() { + let _guard = EnvGuard::set("NODEDB_DEBUG_ENDPOINTS_ENABLED", "TRUE!"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_DEBUG_ENDPOINTS_ENABLED", + "TRUE!", + ); +} + +/// A collector address with no scheme cannot be dialed as an HTTP endpoint. +#[test] +fn endpoint_without_scheme_fails_startup() { + let _guard = EnvGuard::set("NODEDB_OTLP_EXPORT_ENDPOINT", "collector.internal:4318"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_OTLP_EXPORT_ENDPOINT", + "collector.internal:4318", + ); +} diff --git a/nodedb/tests/config_env_sizing.rs b/nodedb/tests/config_env_sizing.rs new file mode 100644 index 000000000..2096abd00 --- /dev/null +++ b/nodedb/tests/config_env_sizing.rs @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Startup rejection of malformed sizing and admission overrides. +//! +//! These variables decide how much of the machine the process takes. A value +//! that fails to parse stops the boot. The compiled defaults are +//! host-dependent, so a fallback runs the node at an unpredictable capacity. + +mod support; + +use nodedb::ServerConfig; +use nodedb::config::server::apply_env_overrides; +use support::env_guard::{EnvGuard, assert_rejected}; + +#[test] +fn malformed_data_plane_cores_fails_startup() { + let _guard = EnvGuard::set("NODEDB_DATA_PLANE_CORES", "abc"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_DATA_PLANE_CORES", + "abc", + ); +} + +/// Zero parses as a `usize` and asks the Data Plane for a shard count no +/// query can reach. A core count is strictly positive. The gate rejects it +/// the way it rejects a non-numeric value. +#[test] +fn zero_data_plane_cores_fails_startup() { + let _guard = EnvGuard::set("NODEDB_DATA_PLANE_CORES", "0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_DATA_PLANE_CORES", + "0", + ); +} + +#[test] +fn negative_data_plane_cores_fails_startup() { + let _guard = EnvGuard::set("NODEDB_DATA_PLANE_CORES", "-4"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_DATA_PLANE_CORES", + "-4", + ); +} + +#[test] +fn malformed_max_connections_fails_startup() { + let _guard = EnvGuard::set("NODEDB_MAX_CONNECTIONS", "many"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_MAX_CONNECTIONS", + "many", + ); +} + +/// The TOML path documents log format as rejected at startup with no silent +/// fallback. The environment path is the same setting and owes the operator +/// the same contract. +#[test] +fn unknown_log_format_fails_startup() { + let _guard = EnvGuard::set("NODEDB_LOG_FORMAT", "logfmt"); + let mut cfg = ServerConfig::default(); + assert_rejected(apply_env_overrides(&mut cfg), "NODEDB_LOG_FORMAT", "logfmt"); +} + +#[test] +fn malformed_timeseries_memtable_budget_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TS_MEMTABLE_BUDGET_BYTES", "64MiB"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TS_MEMTABLE_BUDGET_BYTES", + "64MiB", + ); +} + +#[test] +fn zero_timeseries_memtable_budget_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TS_MEMTABLE_BUDGET_BYTES", "0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TS_MEMTABLE_BUDGET_BYTES", + "0", + ); +} + +#[test] +fn malformed_timeseries_memtable_hard_limit_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES", "unbounded"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES", + "unbounded", + ); +} + +#[test] +fn zero_timeseries_memtable_hard_limit_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES", "0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES", + "0", + ); +} + +#[test] +fn malformed_timeseries_tag_cardinality_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TS_MAX_TAG_CARDINALITY", "100k"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TS_MAX_TAG_CARDINALITY", + "100k", + ); +} + +#[test] +fn zero_timeseries_tag_cardinality_fails_startup() { + let _guard = EnvGuard::set("NODEDB_TS_MAX_TAG_CARDINALITY", "0"); + let mut cfg = ServerConfig::default(); + assert_rejected( + apply_env_overrides(&mut cfg), + "NODEDB_TS_MAX_TAG_CARDINALITY", + "0", + ); +} diff --git a/nodedb/tests/support/env_guard.rs b/nodedb/tests/support/env_guard.rs new file mode 100644 index 000000000..ece78d19f --- /dev/null +++ b/nodedb/tests/support/env_guard.rs @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Env-var scoping and the shared rejection assertion for the startup +//! override tests. +//! +//! Every `NODEDB_*` override is an operator-supplied startup value. A value +//! that fails to parse names sizing, a listener, or a durability setting the +//! process cannot provide. The process refuses to start and names what it +//! refused. A fallback runs the node on a configuration nobody chose. + +#![allow(dead_code)] // Not every test binary needs every helper here. + +/// Sets env vars for the duration of one test and removes them on drop. +/// +/// Env vars are process-global. Each test gets its own process under +/// `cargo nextest run`, which the workspace mandates, so one guard per test +/// isolates enough. The drop still runs, so a panicking test leaves nothing +/// behind for a same-process runner. +pub struct EnvGuard { + keys: Vec, +} + +impl EnvGuard { + /// Sets one variable for the lifetime of the guard. + pub fn set(var: &str, value: &str) -> Self { + unsafe { std::env::set_var(var, value) }; + Self { + keys: vec![var.to_string()], + } + } + + /// Sets several variables for the lifetime of the guard. + pub fn set_all(pairs: &[(&str, &str)]) -> Self { + let mut keys = Vec::with_capacity(pairs.len()); + for (var, value) in pairs { + unsafe { std::env::set_var(var, value) }; + keys.push((*var).to_string()); + } + Self { keys } + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + for key in &self.keys { + unsafe { std::env::remove_var(key) }; + } + } +} + +/// Asserts that an override pass refused `var=value` and said so. +/// +/// The two `contains` checks guard against the silent fallback this covers. +/// An error naming neither the variable nor the value reads like a +/// warn-and-continue log. The operator gets no way to find the typo. +pub fn assert_rejected(result: nodedb::Result<()>, var: &str, value: &str) { + let err = match result { + Ok(()) => panic!( + "{var}={value} was accepted; a malformed startup value must fail startup, \ + not fall back to the config value or the compiled default" + ), + Err(e) => e, + }; + let msg = err.to_string(); + assert!(msg.contains(var), "error must name the variable: {msg}"); + assert!( + msg.contains(value), + "error must name the rejected value: {msg}" + ); +} diff --git a/nodedb/tests/support/mod.rs b/nodedb/tests/support/mod.rs index ab8ef633a..24521b2ca 100644 --- a/nodedb/tests/support/mod.rs +++ b/nodedb/tests/support/mod.rs @@ -1,4 +1,5 @@ // SPDX-License-Identifier: BUSL-1.1 pub mod direct_io; +pub mod env_guard; pub mod memory; From 28d9200eaf888e2cda5997bd90b5f6eb2b75009e Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Mon, 7 Sep 2026 04:08:54 +0800 Subject: [PATCH 2/4] feat(config): expand ${VAR} placeholders in TOML config files Lets operators reference environment variables directly in a config file's TOML source, substituted before parsing. Comments and escaped $${NAME} are left untouched, and an unset or malformed placeholder fails config load instead of falling through to a TOML parse error. Values applied later via NODEDB_* env overrides still take precedence over an expanded placeholder. --- nodedb/src/config/server/config.rs | 103 ++++++- nodedb/src/config/server/env_expand.rs | 372 +++++++++++++++++++++++ nodedb/src/config/server/mod.rs | 3 + nodedb/src/config/server/test_support.rs | 17 ++ 4 files changed, 484 insertions(+), 11 deletions(-) create mode 100644 nodedb/src/config/server/env_expand.rs create mode 100644 nodedb/src/config/server/test_support.rs diff --git a/nodedb/src/config/server/config.rs b/nodedb/src/config/server/config.rs index f1bd41838..aac4351e9 100644 --- a/nodedb/src/config/server/config.rs +++ b/nodedb/src/config/server/config.rs @@ -126,6 +126,11 @@ impl ServerConfig { let content = std::fs::read_to_string(path).map_err(|e| crate::Error::Config { detail: format!("failed to read config file {}: {e}", path.display()), })?; + // A `${NODEDB_*}` placeholder here reads the same variable + // `apply_env_overrides` reads later. The override still wins for + // that field — this is textual substitution before parsing, not a + // second, competing expansion. + let content = super::env_expand::expand_env(path, &content)?; let parsed: Self = toml::from_str(&content).map_err(|e| crate::Error::Config { detail: format!("invalid TOML config: {e}"), })?; @@ -209,7 +214,9 @@ impl ServerConfig { #[cfg(test)] mod tests { use super::*; + use crate::config::server::apply_env_overrides; use crate::config::server::log_format::LogFormat; + use crate::config::server::test_support::with_var; use std::net::{IpAddr, Ipv4Addr}; #[test] @@ -312,23 +319,59 @@ mod tests { assert_eq!(cfg.sync_addr().ip(), IpAddr::V4(Ipv4Addr::new(10, 0, 0, 5))); } - #[test] - fn unknown_top_level_table_rejected() { - // The misplaced `[server_typo]` table must surface, not be silently ignored. - let raw = "[server]\n\n[server_typo]\nfoo = 1\n"; - let err = toml::from_str::(raw).unwrap_err().to_string(); - assert!( - err.contains("unknown field") || err.contains("server_typo"), - "unexpected error: {err}" - ); - } - fn write_temp_config(name: &str, contents: &str) -> std::path::PathBuf { let path = std::env::temp_dir().join(name); std::fs::write(&path, contents).expect("write temp config"); path } + #[test] + fn from_file_expands_a_quoted_string_placeholder() { + with_var( + "NODEDB_TEST_DATA_DIR_PLACEHOLDER", + "/var/lib/nodedb-test", + || { + let path = write_temp_config( + "nodedb-env-expand-quoted.toml", + "[server]\ndata_dir = \"${NODEDB_TEST_DATA_DIR_PLACEHOLDER}\"\n", + ); + let cfg = ServerConfig::from_file(&path).expect("load config"); + std::fs::remove_file(&path).ok(); + assert_eq!( + cfg.server.data_dir, + std::path::PathBuf::from("/var/lib/nodedb-test") + ); + }, + ); + } + + #[test] + fn from_file_expands_an_unquoted_numeric_placeholder() { + with_var("NODEDB_TEST_PGWIRE_PLACEHOLDER", "16432", || { + let path = write_temp_config( + "nodedb-env-expand-numeric.toml", + "[server.ports]\npgwire = ${NODEDB_TEST_PGWIRE_PLACEHOLDER}\n", + ); + let cfg = ServerConfig::from_file(&path).expect("load config"); + std::fs::remove_file(&path).ok(); + assert_eq!(cfg.server.ports.pgwire, 16432); + }); + } + + #[test] + fn from_file_unset_var_fails_before_the_toml_parse() { + unsafe { std::env::remove_var("NODEDB_TEST_UNSET_PLACEHOLDER") }; + let path = write_temp_config( + "nodedb-env-expand-unset.toml", + "[server]\ndata_dir = \"${NODEDB_TEST_UNSET_PLACEHOLDER}\"\n", + ); + let err = ServerConfig::from_file(&path).unwrap_err(); + std::fs::remove_file(&path).ok(); + let msg = err.to_string(); + assert!(msg.contains("NODEDB_TEST_UNSET_PLACEHOLDER"), "{msg}"); + assert!(!msg.contains("invalid TOML config"), "{msg}"); + } + /// The environment gate rejects a zero core count. A TOML file reaches the /// same field without passing that gate, so the bound holds here too. #[test] @@ -377,4 +420,42 @@ mod tests { fn the_compiled_defaults_are_in_domain() { ServerConfig::default().validate().expect("defaults valid"); } + + #[test] + fn from_file_env_override_wins_over_an_expanded_value() { + with_var( + "NODEDB_TEST_OVERRIDE_PLACEHOLDER", + "/from-placeholder", + || { + with_var("NODEDB_DATA_DIR", "/from-override", || { + let path = write_temp_config( + "nodedb-env-expand-override.toml", + "[server]\ndata_dir = \"${NODEDB_TEST_OVERRIDE_PLACEHOLDER}\"\n", + ); + let mut cfg = ServerConfig::from_file(&path).expect("load config"); + std::fs::remove_file(&path).ok(); + assert_eq!( + cfg.server.data_dir, + std::path::PathBuf::from("/from-placeholder") + ); + apply_env_overrides(&mut cfg).expect("apply overrides"); + assert_eq!( + cfg.server.data_dir, + std::path::PathBuf::from("/from-override") + ); + }); + }, + ); + } + + #[test] + fn unknown_top_level_table_rejected() { + // The misplaced `[server_typo]` table must surface, not be silently ignored. + let raw = "[server]\n\n[server_typo]\nfoo = 1\n"; + let err = toml::from_str::(raw).unwrap_err().to_string(); + assert!( + err.contains("unknown field") || err.contains("server_typo"), + "unexpected error: {err}" + ); + } } diff --git a/nodedb/src/config/server/env_expand.rs b/nodedb/src/config/server/env_expand.rs new file mode 100644 index 000000000..db07783db --- /dev/null +++ b/nodedb/src/config/server/env_expand.rs @@ -0,0 +1,372 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! `${VAR}` expansion for a TOML config file, applied before parsing. +//! +//! An operator writes `${NAME}` anywhere in the file. The process substitutes +//! the variable's value as raw text before `toml::from_str` runs. `$${NAME}` +//! escapes to the literal `${NAME}` with no lookup. A placeholder inside a +//! TOML comment stays untouched. A shipped template can therefore carry a +//! commented-out example line without requiring the variable it names. + +use std::path::Path; + +/// Which lexical region the scanner is inside. Only `Normal` and the string +/// states expand placeholders. `Comment` copies text through unchanged. +#[derive(Clone, Copy, PartialEq, Eq)] +enum State { + Normal, + BasicString, + MultilineBasicString, + LiteralString, + MultilineLiteralString, + Comment, +} + +/// Expands every `${NAME}` placeholder in `raw` against the process +/// environment, skipping TOML comments and treating `$${NAME}` as a +/// literal escape. +/// +/// `path` names the config file in violation messages. Every violation +/// found is collected and returned together as one [`crate::Error::Config`]; +/// `Ok` carries the fully expanded text ready for `toml::from_str`. +pub(crate) fn expand_env(path: &Path, raw: &str) -> crate::Result { + let mut out = String::with_capacity(raw.len()); + let mut violations = Vec::new(); + let mut substituted = Vec::new(); + let mut state = State::Normal; + + // Scans `raw` by byte offset, never by collecting it into a `Vec`. + // `i` always sits on a char boundary because every step advances by + // exactly one char's `len_utf8()` or by a fixed ASCII literal length. + let mut i = 0; + while i < raw.len() { + let Some(c) = raw[i..].chars().next() else { + break; + }; + + // Every expandable state (all but `Comment`) hands a `$` to the same + // handler. Checked once here instead of once per state arm below. + if c == '$' && state != State::Comment { + i = handle_dollar(raw, i, path, &mut out, &mut violations, &mut substituted); + continue; + } + + match state { + State::Normal => { + if raw[i..].starts_with("\"\"\"") { + out.push_str("\"\"\""); + state = State::MultilineBasicString; + i += 3; + continue; + } + if raw[i..].starts_with("'''") { + out.push_str("'''"); + state = State::MultilineLiteralString; + i += 3; + continue; + } + if c == '"' { + out.push(c); + state = State::BasicString; + i += c.len_utf8(); + continue; + } + if c == '\'' { + out.push(c); + state = State::LiteralString; + i += c.len_utf8(); + continue; + } + if c == '#' { + out.push(c); + state = State::Comment; + i += c.len_utf8(); + continue; + } + out.push(c); + i += c.len_utf8(); + } + State::Comment => { + out.push(c); + i += c.len_utf8(); + if c == '\n' { + state = State::Normal; + } + } + State::BasicString => { + if c == '\\' + && let Some(next) = raw[i + c.len_utf8()..].chars().next() + { + out.push(c); + out.push(next); + i += c.len_utf8() + next.len_utf8(); + continue; + } + if c == '"' { + out.push(c); + state = State::Normal; + i += c.len_utf8(); + continue; + } + out.push(c); + i += c.len_utf8(); + } + State::MultilineBasicString => { + if c == '\\' + && let Some(next) = raw[i + c.len_utf8()..].chars().next() + { + out.push(c); + out.push(next); + i += c.len_utf8() + next.len_utf8(); + continue; + } + if raw[i..].starts_with("\"\"\"") { + out.push_str("\"\"\""); + state = State::Normal; + i += 3; + continue; + } + out.push(c); + i += c.len_utf8(); + } + State::LiteralString => { + if c == '\'' { + out.push(c); + state = State::Normal; + i += c.len_utf8(); + continue; + } + out.push(c); + i += c.len_utf8(); + } + State::MultilineLiteralString => { + if raw[i..].starts_with("'''") { + out.push_str("'''"); + state = State::Normal; + i += 3; + continue; + } + out.push(c); + i += c.len_utf8(); + } + } + } + + if !violations.is_empty() { + return Err(crate::Error::Config { + detail: violations.join("; "), + }); + } + + substituted.sort_unstable(); + substituted.dedup(); + if !substituted.is_empty() { + tracing::info!( + config_file = %path.display(), + vars = ?substituted, + "expanded ${{VAR}} placeholders in config file" + ); + } + + Ok(out) +} + +/// Handles a `$` found at byte offset `i` in an expandable state. `$${NAME}` +/// escapes to a literal `${NAME}`, and `${NAME}` expands. Anything else is a +/// violation, or ordinary text passed through. Returns the next byte offset. +fn handle_dollar( + raw: &str, + i: usize, + path: &Path, + out: &mut String, + violations: &mut Vec, + substituted: &mut Vec, +) -> usize { + if raw[i..].starts_with("$${") { + if let Some((name, end)) = read_placeholder_name(raw, i + 3) { + out.push('$'); + out.push('{'); + out.push_str(name); + out.push('}'); + return end; + } + violations.push(format!( + "unterminated ${{...}} placeholder in {}", + path.display() + )); + return raw.len(); + } + + if raw[i..].starts_with("${") { + match read_placeholder_name(raw, i + 2) { + Some((name, end)) => { + if !is_valid_name(name) { + violations.push(format!( + "invalid placeholder name '${{{name}}}' in {}: expected [A-Za-z_][A-Za-z0-9_]*", + path.display() + )); + return end; + } + match std::env::var(name) { + Ok(value) => { + out.push_str(&value); + substituted.push(name.to_string()); + } + Err(_) => { + violations.push(format!( + "unset environment variable '{name}' referenced as ${{{name}}} in {}", + path.display() + )); + } + } + end + } + None => { + violations.push(format!( + "unterminated ${{...}} placeholder in {}", + path.display() + )); + raw.len() + } + } + } else { + out.push('$'); + i + 1 + } +} + +/// Reads the `NAME` (and trailing `}`) of a `${NAME}` starting right after +/// the opening `{`. `start` is the byte offset of the first character of the +/// name. Returns the unvalidated name as a borrowed slice of `raw`, and the +/// byte offset past the closing `}`. Returns `None` when the file ends +/// before a `}`. +fn read_placeholder_name(raw: &str, start: usize) -> Option<(&str, usize)> { + let rest = raw.get(start..)?; + let end_in_rest = rest.find('}')?; + Some((&rest[..end_in_rest], start + end_in_rest + 1)) +} + +/// `[A-Za-z_][A-Za-z0-9_]*`, matched over the whole name. +fn is_valid_name(name: &str) -> bool { + let mut chars = name.chars(); + match chars.next() { + Some(c) if c.is_ascii_alphabetic() || c == '_' => {} + _ => return false, + } + chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::server::test_support::with_var; + use std::path::PathBuf; + + fn p() -> PathBuf { + PathBuf::from("/etc/nodedb/config.toml") + } + + #[test] + fn plain_var_expands_to_value() { + with_var("ENV_EXPAND_TEST_PLAIN", "hello", || { + let out = expand_env(&p(), "x = \"${ENV_EXPAND_TEST_PLAIN}\"").unwrap(); + assert_eq!(out, "x = \"hello\""); + }); + } + + #[test] + fn unset_var_is_a_violation() { + unsafe { std::env::remove_var("ENV_EXPAND_TEST_UNSET") }; + let err = expand_env(&p(), "x = ${ENV_EXPAND_TEST_UNSET}").unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("ENV_EXPAND_TEST_UNSET"), "{msg}"); + assert!(msg.contains("/etc/nodedb/config.toml"), "{msg}"); + } + + #[test] + fn malformed_name_is_a_violation() { + for raw in ["x = ${1BAD}", "x = ${}", "x = ${FOO-BAR}"] { + let err = expand_env(&p(), raw).unwrap_err(); + assert!(err.to_string().contains("invalid"), "{raw}: {err}"); + } + } + + #[test] + fn unterminated_placeholder_is_a_violation() { + let err = expand_env(&p(), "x = ${FOO").unwrap_err(); + assert!(err.to_string().contains("unterminated"), "{err}"); + } + + #[test] + fn escaped_dollar_yields_literal() { + unsafe { std::env::remove_var("FOO") }; + let out = expand_env(&p(), "x = \"$${FOO}\"").unwrap(); + assert_eq!(out, "x = \"${FOO}\""); + } + + #[test] + fn multiple_violations_collected_together() { + let err = expand_env(&p(), "a = ${1BAD}\nb = ${2BAD}").unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("1BAD"), "{msg}"); + assert!(msg.contains("2BAD"), "{msg}"); + } + + #[test] + fn placeholder_in_a_comment_is_left_alone() { + unsafe { std::env::remove_var("ENV_EXPAND_TEST_COMMENT_VAR") }; + let raw = "# data_dir = \"${ENV_EXPAND_TEST_COMMENT_VAR}\"\n"; + let out = expand_env(&p(), raw).unwrap(); + assert_eq!(out, raw); + } + + #[test] + fn unset_placeholder_in_a_comment_does_not_fail() { + unsafe { std::env::remove_var("ENV_EXPAND_TEST_COMMENT_UNSET") }; + let raw = "# example: ${ENV_EXPAND_TEST_COMMENT_UNSET}\nreal = 1\n"; + assert!(expand_env(&p(), raw).is_ok()); + } + + #[test] + fn hash_inside_a_string_is_not_a_comment() { + with_var("ENV_EXPAND_TEST_HASH", "value", || { + let raw = "x = \"#${ENV_EXPAND_TEST_HASH}\""; + let out = expand_env(&p(), raw).unwrap(); + assert_eq!(out, "x = \"#value\""); + }); + } + + #[test] + fn placeholder_in_a_multiline_string_expands() { + with_var("ENV_EXPAND_TEST_MULTILINE", "mval", || { + let raw = "x = \"\"\"line ${ENV_EXPAND_TEST_MULTILINE} end\"\"\""; + let out = expand_env(&p(), raw).unwrap(); + assert_eq!(out, "x = \"\"\"line mval end\"\"\""); + }); + } + + #[test] + fn substituted_value_containing_a_placeholder_is_not_re_expanded() { + with_var("ENV_EXPAND_TEST_OUTER", "${ENV_EXPAND_TEST_INNER}", || { + unsafe { std::env::remove_var("ENV_EXPAND_TEST_INNER") }; + let out = expand_env(&p(), "x = \"${ENV_EXPAND_TEST_OUTER}\"").unwrap(); + assert_eq!(out, "x = \"${ENV_EXPAND_TEST_INNER}\""); + }); + } + + #[test] + fn unicode_text_around_a_placeholder_is_preserved() { + with_var("ENV_EXPAND_TEST_UNICODE", "val", || { + let raw = "name = \"caf\u{e9} \u{1f600} ${ENV_EXPAND_TEST_UNICODE} \u{4e2d}\u{6587}\""; + let out = expand_env(&p(), raw).unwrap(); + assert_eq!(out, "name = \"caf\u{e9} \u{1f600} val \u{4e2d}\u{6587}\""); + }); + } + + #[test] + fn text_without_placeholders_is_unchanged() { + let raw = "[server]\nhost = \"0.0.0.0\"\nport = 6432\n"; + let out = expand_env(&p(), raw).unwrap(); + assert_eq!(out, raw); + } +} diff --git a/nodedb/src/config/server/mod.rs b/nodedb/src/config/server/mod.rs index ccd331b12..a5bac6b5e 100644 --- a/nodedb/src/config/server/mod.rs +++ b/nodedb/src/config/server/mod.rs @@ -6,6 +6,7 @@ mod cold_storage; mod config; mod domain; mod env; +mod env_expand; mod log_format; mod observability; mod paths; @@ -14,6 +15,8 @@ mod retention; pub mod scheduler; mod section; mod snapshot_storage; +#[cfg(test)] +mod test_support; mod tls; pub use checkpoint::CheckpointSettings; diff --git a/nodedb/src/config/server/test_support.rs b/nodedb/src/config/server/test_support.rs new file mode 100644 index 000000000..3911b3747 --- /dev/null +++ b/nodedb/src/config/server/test_support.rs @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Shared env-var test helper for `config/server` unit tests. +//! +//! Env vars are process-global, so a test that sets one must always remove +//! it, even when the test body panics. + +/// Sets an env var for the duration of `f`, then always removes it. +pub(crate) fn with_var(name: &str, value: &str, f: impl FnOnce() -> R) -> R { + unsafe { std::env::set_var(name, value) }; + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)); + unsafe { std::env::remove_var(name) }; + match result { + Ok(r) => r, + Err(payload) => std::panic::resume_unwind(payload), + } +} From e681470eca5a4d3b69e5216b441aba151c1f5fc0 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Mon, 7 Sep 2026 04:08:54 +0800 Subject: [PATCH 3/4] docs(config): document startup validation and env placeholders Cover the new config surface: sync_host, TLS certificate material, WAL tuning, cluster settings, maintenance intervals, and observability options, plus the startup-validation error format and ${VAR} placeholder expansion in TOML files. --- docs/getting-started.md | 142 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 132 insertions(+), 10 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index 831c07563..7f2ab69f9 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -12,7 +12,7 @@ All three share the same [configuration](#configuration), [connection](#connect) ## Run with Docker -The easiest way to get started, and the right choice on macOS, Windows, or any host where you don't want to manage a binary directly. On native Linux, the [prebuilt binary](#run-a-prebuilt-binary-linux) gives you better performance. +The quickest start, and the right choice on macOS and Windows. Also the right choice on any host where you avoid managing a binary. On native Linux, the [prebuilt binary](#run-a-prebuilt-binary-linux) gives you better performance. ### Docker Compose @@ -50,7 +50,7 @@ docker run -d --name nodedb \ Sync (9090) is omitted: it binds to loopback, so a mapping cannot reach it. See [Protocols](protocols.md). -The container entrypoint runs as root just long enough to fix ownership on the data volume, then drops privileges to the `nodedb` user (uid 10001). To skip the root step, pass `--user 10001:10001` and pre-create the volume with matching ownership. +The container entrypoint runs as root only to fix ownership on the data volume. It then drops privileges to the `nodedb` user (uid 10001). To skip the root step, pass `--user 10001:10001` and pre-create the volume with matching ownership. ### Default ports @@ -91,7 +91,7 @@ Set them under `environment:` in `docker-compose.yml` or pass with `-e` to `dock ## Run a prebuilt binary (Linux) -Each tagged release ships a static `nodedb` tarball on GitHub for `linux-x64` and `linux-arm64`. macOS and Windows users should use Docker until those targets ship. +Each tagged release ships a static `nodedb` tarball on GitHub for `linux-x64` and `linux-arm64`. macOS and Windows users run Docker until those targets ship. ```bash # Resolve the latest tag and your architecture @@ -186,11 +186,13 @@ Requires Rust 1.94+ and Linux (the Data Plane uses io_uring). The build produces ## Configuration -This section applies to **every** install method — Docker, prebuilt binary, and source builds all read the same TOML schema and respond to the same environment variables. Pick whichever is convenient: +This section applies to **every** install method. Docker, prebuilt binary, and source builds read the same TOML schema and the same environment variables. Pick whichever is convenient: -- **TOML file** — pass `--config /path/to/nodedb.toml` on the command line. Best for production / systemd / pre-baked images. +- **TOML file** — pass `--config /path/to/nodedb.toml` on the command line. Best for production / systemd / pre-baked images. A value can read the environment with `${VAR}` — see [Placeholders in the config file](#placeholders-in-the-config-file). - **Environment variables** — prefix `NODEDB_*`. Best for Docker (`-e`), Compose (`environment:`), and Kubernetes. Env vars **override** values from the TOML file when both are set. +A value NodeDB cannot apply stops startup. See [Startup validation](#startup-validation). + ### Default ports By default, NodeDB listens on: @@ -248,6 +250,7 @@ port named — the server never comes up missing a protocol. | Config field | Environment variable | Default | | ------------------ | ------------------------- | ----------------------------------------------------- | | `host` | `NODEDB_HOST` | `127.0.0.1` | +| `sync_host` | `NODEDB_SYNC_HOST` | follows `host` when `host` is loopback | | `ports.native` | `NODEDB_PORT_NATIVE` | `6433` | | `ports.pgwire` | `NODEDB_PORT_PGWIRE` | `6432` | | `ports.http` | `NODEDB_PORT_HTTP` | `6480` | @@ -260,6 +263,17 @@ port named — the server never comes up missing a protocol. | `max_connections` | `NODEDB_MAX_CONNECTIONS` | `4096` | | `log_format` | `NODEDB_LOG_FORMAT` | `text` | +**TLS certificate material:** + +| Config field | Environment variable | Default | +| --------------- | ------------------------ | ------- | +| `tls.cert_path` | `NODEDB_TLS_CERT_PATH` | none | +| `tls.key_path` | `NODEDB_TLS_KEY_PATH` | none | + +Setting both creates a `[server.tls]` section when the config file has none. +Every protocol then starts with TLS on. Setting one without the other stops +startup. + **Per-protocol TLS** (only applies when `[server.tls]` is configured): | Config field | Environment variable | Default | @@ -270,12 +284,23 @@ port named — the server never comes up missing a protocol. | `tls.resp` | `NODEDB_TLS_RESP` | `true` | | `tls.ilp` | `NODEDB_TLS_ILP` | `true` | +Every toggle on this page accepts `true`, `1`, `yes`, `false`, `0`, and `no`, +in any case. Turning a listener on without certificate material stops startup. +Turning one off with no `[server.tls]` section is accepted, because the +listener is already plaintext. + **Checkpoint & WAL settings:** | Config field | Environment variable | Default | | -------------------------------- | ---------------------------------- | ------- | | `checkpoint.interval_secs` | `NODEDB_CHECKPOINT_INTERVAL_SECS` | `300` | | `checkpoint.wal_segment_target_mb` | `NODEDB_WAL_SEGMENT_TARGET_MB` | `64` | +| `tuning.wal.direct_io` | `NODEDB_WAL_DIRECT_IO` | `true` | +| `tuning.wal.write_buffer_size` | `NODEDB_WAL_WRITE_BUFFER_SIZE` | `2MiB` | + +Both intervals must be positive. `write_buffer_size` accepts a memory size and +must be at least `64KiB`. Turn `direct_io` off only on a filesystem that +rejects `O_DIRECT`. **Timeseries memtable settings:** @@ -285,13 +310,110 @@ port named — the server never comes up missing a protocol. | `tuning.timeseries.memtable_hard_limit_bytes` | `NODEDB_TS_MEMTABLE_HARD_LIMIT_BYTES` | `83886080` (80 MiB) | | `tuning.timeseries.max_tag_cardinality` | `NODEDB_TS_MAX_TAG_CARDINALITY` | `100000` | -`memtable_budget_bytes` is the soft budget that schedules a flush; +`memtable_budget_bytes` is the soft budget that schedules a flush. `memtable_hard_limit_bytes` is the ceiling that forces one before the next -write is applied. A single write is always applied whole, so it can carry the -memtable past the hard limit by its own size before the next flush drains it. -`max_tag_cardinality` bounds the distinct values a text/tag column may hold +write. A single write always applies whole, so it can carry the memtable past +the hard limit by its own size. The next flush then drains it. +`max_tag_cardinality` bounds the distinct values a text/tag column holds between flushes. +**Cluster settings** (each needs a `[cluster]` section in the config file): + +| Config field | Environment variable | Default | +| ------------------------------------- | ------------------------------------ | ------- | +| `cluster.node_id` | `NODEDB_NODE_ID` | none | +| `cluster.seed_nodes` | `NODEDB_SEED_NODES` | none | +| `cluster.join_retry_max_attempts` | `NODEDB_JOIN_RETRY_MAX_ATTEMPTS` | `8` | +| `cluster.join_retry_max_backoff_secs` | `NODEDB_JOIN_RETRY_MAX_BACKOFF_SECS` | `32` | + +`NODEDB_SEED_NODES` takes a comma-separated `host:port` list. Both join-retry +values must be positive. Setting any of these without a `[cluster]` section +stops startup. + +**Maintenance loop settings:** + +| Config field | Environment variable | Default | +| ----------------------------------------------------- | ----------------------------------------- | ------- | +| `tuning.maintenance.clone_sweep_interval_ms` | `NODEDB_CLONE_SWEEP_INTERVAL_MS` | `30000` | +| `tuning.maintenance.constraint_reconcile_interval_ms` | `NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS` | `1000` | +| `tuning.maintenance.scope_expiry_interval_secs` | `NODEDB_SCOPE_EXPIRY_INTERVAL_SECS` | `60` | + +All three must be positive. `scope_expiry_interval_secs` has a floor of `10`. +Below that the sweep costs more than the resolution it buys. + +**Observability settings:** + +| Config field | Environment variable | Default | +| ------------------------------------------------- | --------------------------------- | -------------- | +| `observability.promql.enabled` | `NODEDB_PROMQL_ENABLED` | `true` | +| `observability.otlp.receiver.enabled` | `NODEDB_OTLP_RECEIVER_ENABLED` | `false` | +| `observability.otlp.receiver.http_listen` | `NODEDB_OTLP_HTTP_LISTEN` | `0.0.0.0:4318` | +| `observability.otlp.receiver.grpc_listen` | `NODEDB_OTLP_GRPC_LISTEN` | `0.0.0.0:4317` | +| `observability.otlp.export.enabled` | `NODEDB_OTLP_EXPORT_ENABLED` | `false` | +| `observability.otlp.export.endpoint` | `NODEDB_OTLP_EXPORT_ENDPOINT` | none | +| `observability.otlp.export.metrics_interval_secs` | `NODEDB_OTLP_EXPORT_INTERVAL` | `15` | +| `observability.debug_endpoints_enabled` | `NODEDB_DEBUG_ENDPOINTS_ENABLED` | `false` | + +`NODEDB_OTLP_EXPORT_ENDPOINT` takes an `http://` or `https://` URL. The debug +endpoints expose raft internals, so they stay off until you enable them. + +### Startup validation + +A `NODEDB_*` value NodeDB cannot apply stops startup. The server reports every +bad value at once and exits non-zero. + +Three kinds fail: + +- **Unparseable** — `NODEDB_DATA_PLANE_CORES=abc`. +- **Out of domain** — a zero core count, or a WAL buffer under `64KiB`. +- **Unsatisfiable here** — TLS on with no certificate material, or + `NODEDB_NODE_ID` with no `[cluster]` section. + +``` +Error: configuration error: invalid value 'abc' for NODEDB_DATA_PLANE_CORES: expected a positive integer; invalid value '4096' for NODEDB_WAL_WRITE_BUFFER_SIZE: expected a memory size of at least 64KiB +``` + +A request the server already satisfies is honoured. `NODEDB_TLS_PGWIRE=false` +with no `[server.tls]` section is accepted, because that listener is already +plaintext. + +An empty value is always a violation. `NODEDB_DATA_DIR=` is a failed template +substitution, not a request. + +### Placeholders in the config file + +A config value can read the environment with `${VAR}`. NodeDB substitutes +before it parses the TOML. + +```toml +[server] +data_dir = "${DATA_DIR}" + +[server.ports] +pgwire = ${PGWIRE_PORT} +``` + +```yaml +services: + nodedb: + environment: + DATA_DIR: /var/lib/nodedb + PGWIRE_PORT: "6432" +``` + +- **Quoting is yours** — substitution is textual. Quote for a string, leave + bare for a number or a bool. +- **Escape** — `$${NAME}` produces the literal `${NAME}` and reads nothing. +- **Comments are skipped** — a commented-out example never requires its + variable. +- **Names** match `[A-Za-z_][A-Za-z0-9_]*`. +- **Unset fails** — the error names the file and the variable. +- **No shell syntax** — no command substitution, no recursion, no + `${VAR:-default}`. +- **Never logged** — NodeDB logs the variable name, never the value. + +An env override still wins over an expanded value for the same field. + ## Connect ### With the `ndb` CLI @@ -312,7 +434,7 @@ NodeDB speaks PostgreSQL's wire protocol, so standard tools like `psql`, ORMs, a ### With the Rust SDK or FFI -The `nodedb-client` crate connects over the NDB protocol (port 6433) and supports both SQL and native modes on the same connection: +The `nodedb-client` crate connects over the NDB protocol (port 6433). One connection carries both SQL and native modes: ```rust // SQL — same as psql/HTTP, full query support From fc7cc0047dcc21614b224bbe68df363fef763510 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Mon, 7 Sep 2026 10:32:43 +0800 Subject: [PATCH 4/4] test(harness): fix flaky pgwire and timeseries/vector inproc tests Reset tenant rate counters each second in the pgwire test harness's poller loop, since the server's own background loop that normally does this is never started for tests, so quotas hit once a test exceeded them. Shrink the memtable budget in the timeseries flush tests instead of ingesting millions of rows to cross the shipped 64 MiB default, which timed out on loaded CI runners. Rework the reindex-concurrent stall bound to scale with the rebuild window's own duration and gate on p99 rather than a single slowest sample, throttle its query load below the tenant's default rate limit, and surface the full error chain from failed queries. --- .../src/pgwire_harness/multicore.rs | 5 ++ .../src/pgwire_harness/restart.rs | 5 ++ .../src/pgwire_harness/start.rs | 5 ++ .../src/pgwire_harness/support.rs | 17 ++++ .../cases/executor_tests/test_timeseries.rs | 19 +++-- .../executor_tests/test_timeseries_budget.rs | 25 +++++- .../inproc/cases/reindex_vector_concurrent.rs | 82 ++++++++++++++----- 7 files changed, 130 insertions(+), 28 deletions(-) diff --git a/nodedb-test-support/src/pgwire_harness/multicore.rs b/nodedb-test-support/src/pgwire_harness/multicore.rs index 93636b0c4..caf2206ee 100644 --- a/nodedb-test-support/src/pgwire_harness/multicore.rs +++ b/nodedb-test-support/src/pgwire_harness/multicore.rs @@ -86,8 +86,13 @@ impl TestServer { let shared_poller = Arc::clone(&shared); let (poller_shutdown_tx, mut poller_shutdown_rx) = tokio::sync::watch::channel(false); let poller_handle = tokio::spawn(async move { + let mut last_rate_reset = std::time::Instant::now(); loop { shared_poller.poll_and_route_responses(); + super::support::reset_tenant_rate_counters_each_second( + &shared_poller, + &mut last_rate_reset, + ); tokio::select! { _ = tokio::time::sleep(Duration::from_millis(1)) => {} _ = poller_shutdown_rx.changed() => break, diff --git a/nodedb-test-support/src/pgwire_harness/restart.rs b/nodedb-test-support/src/pgwire_harness/restart.rs index c00208a91..cc815fcc3 100644 --- a/nodedb-test-support/src/pgwire_harness/restart.rs +++ b/nodedb-test-support/src/pgwire_harness/restart.rs @@ -265,8 +265,13 @@ impl TestServer { let shared_poller = Arc::clone(&shared); let (poller_shutdown_tx, mut poller_shutdown_rx) = tokio::sync::watch::channel(false); let poller_handle = tokio::spawn(async move { + let mut last_rate_reset = std::time::Instant::now(); loop { shared_poller.poll_and_route_responses(); + super::support::reset_tenant_rate_counters_each_second( + &shared_poller, + &mut last_rate_reset, + ); tokio::select! { _ = tokio::time::sleep(Duration::from_millis(1)) => {} _ = poller_shutdown_rx.changed() => break, diff --git a/nodedb-test-support/src/pgwire_harness/start.rs b/nodedb-test-support/src/pgwire_harness/start.rs index e0ae75cba..ccd2044df 100644 --- a/nodedb-test-support/src/pgwire_harness/start.rs +++ b/nodedb-test-support/src/pgwire_harness/start.rs @@ -288,8 +288,13 @@ impl TestServer { let shared_poller = Arc::clone(&shared); let (poller_shutdown_tx, mut poller_shutdown_rx) = tokio::sync::watch::channel(false); let poller_handle = tokio::spawn(async move { + let mut last_rate_reset = std::time::Instant::now(); loop { shared_poller.poll_and_route_responses(); + super::support::reset_tenant_rate_counters_each_second( + &shared_poller, + &mut last_rate_reset, + ); tokio::select! { _ = tokio::time::sleep(Duration::from_millis(1)) => {} _ = poller_shutdown_rx.changed() => break, diff --git a/nodedb-test-support/src/pgwire_harness/support.rs b/nodedb-test-support/src/pgwire_harness/support.rs index 5d5318c64..f150057f5 100644 --- a/nodedb-test-support/src/pgwire_harness/support.rs +++ b/nodedb-test-support/src/pgwire_harness/support.rs @@ -87,3 +87,20 @@ pub(super) async fn bind_http_listener( }); (port, handle) } + +/// Runs the tenant rate-counter reset the server drives from +/// `spawn_background_loops`, which this harness does not start. +/// +/// `requests_this_second` only means "this second" while something clears it. +/// Without this it is a running total, so a tenant is refused for good once a +/// test issues more requests than its quota, and the error names a rate the +/// test never reached. +pub(super) fn reset_tenant_rate_counters_each_second( + shared: &SharedState, + last_reset: &mut std::time::Instant, +) { + if last_reset.elapsed() >= std::time::Duration::from_secs(1) { + shared.reset_tenant_rate_counters(); + *last_reset = std::time::Instant::now(); + } +} diff --git a/nodedb/tests/inproc/cases/executor_tests/test_timeseries.rs b/nodedb/tests/inproc/cases/executor_tests/test_timeseries.rs index e04421940..bc3e9945f 100644 --- a/nodedb/tests/inproc/cases/executor_tests/test_timeseries.rs +++ b/nodedb/tests/inproc/cases/executor_tests/test_timeseries.rs @@ -148,12 +148,21 @@ fn ts_scan_filtered( fn count_star_sees_flushed_partitions() { let mut ctx = make_ctx(); - // Ingest enough wide rows to force multiple memtable flushes. - // Each row is ~7 columns × ~8 bytes = ~56 bytes of column data. - // 64MB / 56 = ~1.2M rows per flush. We send 3M rows to guarantee - // at least 2 flush cycles. Each batch is 10K rows. + // Shrink the memtable budget, then ingest enough wide rows to force + // several flushes. Each row is ~7 columns x ~8 bytes = ~56 bytes, so a + // 1 MiB budget flushes every ~18 K rows and 100 K rows cross several + // cycles. Sizing the workload to the shipped 64 MiB budget instead costs + // three million rows, which a loaded CI runner cannot finish inside the + // harness timeout. + ctx.core + .set_timeseries_tuning(nodedb_types::config::tuning::TimeseriesToning { + memtable_budget_bytes: 1024 * 1024, + memtable_hard_limit_bytes: 4 * 1024 * 1024, + ..Default::default() + }); + let batch_size = 10_000; - let num_batches = 300; + let num_batches = 10; let mut total_accepted: u64 = 0; let mut total_rejected: u64 = 0; diff --git a/nodedb/tests/inproc/cases/executor_tests/test_timeseries_budget.rs b/nodedb/tests/inproc/cases/executor_tests/test_timeseries_budget.rs index a343e1135..77bcf28c8 100644 --- a/nodedb/tests/inproc/cases/executor_tests/test_timeseries_budget.rs +++ b/nodedb/tests/inproc/cases/executor_tests/test_timeseries_budget.rs @@ -104,13 +104,36 @@ fn ingest_ilp(ctx: &mut TestCtx, collection: &str, payload: &str) -> serde_json: /// `count_star_sees_flushed_partitions` uses the same 3 M-row volume to /// guarantee ≥ 2 flushes; we reuse that so these tests fail for the /// accounting bug and not because nothing flushed. +/// Memtable budget for the flush workload, well under the shipped default. +/// +/// At roughly 56 bytes per wide row this flushes every ~18 K rows, so the +/// 100 K-row workload below crosses several cycles and still leaves a +/// resident memtable for the governor to account for. +const FLUSH_BUDGET_BYTES: usize = 1024 * 1024; + +/// Hard limit for the same workload. It sits above the budget so the +/// pre-flush always runs before admission control rejects a row. +const FLUSH_HARD_LIMIT_BYTES: usize = 4 * 1024 * 1024; + fn run_ts_flush_workload() -> (TestCtx, Arc) { let mut ctx = make_ctx(); let gov = generous_governor(); ctx.core.set_governor_for_testing(Arc::clone(&gov)); + // Shrink the memtable budget rather than ingesting past the shipped + // 64 MiB one. The flush path reads `memtable_budget_bytes`, so a small + // budget crosses the same cycles on a fraction of the rows. Sizing the + // workload to the default instead costs three million rows, which a + // loaded CI runner cannot finish inside the harness timeout. + ctx.core + .set_timeseries_tuning(nodedb_types::config::tuning::TimeseriesToning { + memtable_budget_bytes: FLUSH_BUDGET_BYTES, + memtable_hard_limit_bytes: FLUSH_HARD_LIMIT_BYTES, + ..Default::default() + }); + let batch_size = 10_000usize; - let num_batches = 300usize; + let num_batches = 10usize; let mut accepted: u64 = 0; let mut rejected: u64 = 0; for b in 0..num_batches { diff --git a/nodedb/tests/inproc/cases/reindex_vector_concurrent.rs b/nodedb/tests/inproc/cases/reindex_vector_concurrent.rs index ba0db8586..4bf9aeb6f 100644 --- a/nodedb/tests/inproc/cases/reindex_vector_concurrent.rs +++ b/nodedb/tests/inproc/cases/reindex_vector_concurrent.rs @@ -9,8 +9,9 @@ //! actually holds the rebuild to being correct, not merely quick //! 2. no query errored during the rebuild //! 3. queries kept completing throughout the rebuild, not just before it -//! 4. no query stalled past `STALL_BOUND` — the signature of a rebuild -//! that took an exclusive lock instead of running concurrently +//! 4. the query p99 stayed a small share of the rebuild window — the +//! signature of a rebuild that took an exclusive lock instead of +//! running concurrently //! 5. exactly one `atomic_cutover` tracing event was emitted by the //! `nodedb::reindex` target during the rebuild phase //! @@ -22,7 +23,9 @@ //! so the rebuild window holds a few hundred samples, and the assertions below //! are orders of magnitude away from scheduler noise rather than a 2× multiple //! of it. Latencies are still printed, so a real slowdown stays visible in the -//! log without gating CI on wall-clock. +//! log without gating CI on wall-clock. The one remaining timing gate is a +//! share of the rebuild's own duration, so a slower machine widens the bound +//! and the workload it measures together. //! //! Assertion 1 is the load-bearing one and is the only one whose failure mode //! was verified by deliberately breaking it: a query answered out of a @@ -160,19 +163,35 @@ async fn reindex_vector_concurrent_p99() { const ROWS: usize = 1_000; const BATCH: usize = 500; const BASELINE_QUERIES: usize = 20; - // One query per millisecond, so the rebuild window holds many samples - // instead of one. The loop paces itself and skips the sleep when a query - // already took longer than the interval. - const REBUILD_QPS: u64 = 1_000; - - /// Longest a single query may take during the rebuild. + // Query rate during the rebuild. The loop paces itself and skips the sleep + // when a query already took longer than the interval. + // + // A tenant's default quota is 1000 qps (`control::security::tenant`), so + // driving the rebuild at that rate refuses queries as soon as the rebuild + // outlives the limiter's own window. A fast machine rebuilds in a few + // hundred milliseconds and never fills it. A slower one sustains the rate + // for seconds and sees most queries rejected. The limiter is production + // behaviour, so the load generator stays well under it. + const REBUILD_QPS: u64 = 200; + + /// Share of the rebuild window the p99 query may occupy. + /// + /// A rebuild holding an exclusive lock blocks every query it overlaps, so + /// the p99 approaches the window itself. A concurrent rebuild leaves it + /// far below. Both sides scale together when the machine slows down, + /// which an absolute bound does not — a busy CI runner crosses a fixed + /// two seconds while running perfectly concurrently. /// - /// A concurrent rebuild leaves queries in the low milliseconds; one that - /// takes an exclusive lock blocks them for the whole rebuild, which this - /// test already allows up to 60s for. Two seconds sits between those two - /// regimes by orders of magnitude, so a loaded machine cannot cross it but - /// a lock-holding rebuild cannot avoid it. - const STALL_BOUND: Duration = Duration::from_secs(2); + /// The p99 carries the gate rather than the single slowest sample. One + /// descheduled query on an oversubscribed runner moves the maximum and + /// says nothing about locking. + const STALL_SHARE_OF_WINDOW: u32 = 2; + + /// Floor under the derived bound, for a rebuild that finishes in + /// milliseconds. Scheduler noise alone can then exceed half the window, + /// and this keeps the derived bound from being stricter than the absolute + /// one it replaces. + const STALL_FLOOR: Duration = Duration::from_millis(250); /// Fewest queries that must complete during the rebuild window. /// @@ -278,7 +297,19 @@ async fn reindex_vector_concurrent_p99() { empty_writer.fetch_add(1, Ordering::Relaxed); } } - Err(error) => errors_writer.lock().unwrap().push(error.to_string()), + Err(error) => { + // `Display` on a tokio_postgres error is "db error" and + // nothing else. The server's message lives in the source + // chain, and it is the only thing that names the fault. + let mut detail = format!("{error}"); + let mut cause: Option<&(dyn std::error::Error + 'static)> = + std::error::Error::source(&error); + while let Some(inner) = cause { + detail.push_str(&format!(" <- {inner}")); + cause = inner.source(); + } + errors_writer.lock().unwrap().push(detail); + } } lats_writer.lock().unwrap().push(lat); // Pace to target QPS; no-op if query took longer than the interval. @@ -291,6 +322,7 @@ async fn reindex_vector_concurrent_p99() { // Issue REINDEX CONCURRENTLY on the main client. // This returns as soon as the background thread is started; the atomic // cutover is applied on a later tick() — so we must wait for it. + let rebuild_started = Instant::now(); server.exec("REINDEX CONCURRENTLY vecs10k").await.unwrap(); // Wait up to 60 s for the Data Plane to complete the background rebuild @@ -301,6 +333,8 @@ async fn reindex_vector_concurrent_p99() { tokio::time::sleep(Duration::from_millis(5)).await; } + let rebuild_window = rebuild_started.elapsed(); + // Signal the query task to stop and collect its latencies. stop_flag.store(1, Ordering::Relaxed); let _ = query_handle.await; @@ -350,13 +384,17 @@ async fn reindex_vector_concurrent_p99() { rebuild_samples.len() ); + let stall_bound = (rebuild_window / STALL_SHARE_OF_WINDOW).max(STALL_FLOOR); + let rebuild_p99 = p99(rebuild_samples.clone()); assert!( - slowest < STALL_BOUND, - "a query stalled {:.1}ms during the rebuild (bound {:.0}ms) — the \ - signature of a rebuild holding an exclusive lock rather than running \ - concurrently", - slowest.as_secs_f64() * 1000.0, - STALL_BOUND.as_secs_f64() * 1000.0 + rebuild_p99 < stall_bound, + "queries ran at a {:.1}ms p99 during a {:.1}ms rebuild window (bound \ + {:.1}ms, slowest {:.1}ms) — the signature of a rebuild holding an \ + exclusive lock rather than running concurrently", + rebuild_p99.as_secs_f64() * 1000.0, + rebuild_window.as_secs_f64() * 1000.0, + stall_bound.as_secs_f64() * 1000.0, + slowest.as_secs_f64() * 1000.0 ); // Verify exactly one atomic_cutover event was emitted during the rebuild.