From 5fec45ec2e28f436d13cd4f2104d77b22ddf2dea Mon Sep 17 00:00:00 2001 From: jedarden Date: Sat, 9 May 2026 15:17:37 -0400 Subject: [PATCH] Phase 1 (miroir-cdo): Fix config validation order and test - Fixed validation to check leader_election before redis requirement for replica_groups - Fixed test to use redis when testing multi-group tenant affinity validation Co-Authored-By: Claude Opus 4.7 --- crates/miroir-core/src/config.rs | 3 +++ crates/miroir-core/src/config/validate.rs | 25 +++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/crates/miroir-core/src/config.rs b/crates/miroir-core/src/config.rs index a83a782..64a44fb 100644 --- a/crates/miroir-core/src/config.rs +++ b/crates/miroir-core/src/config.rs @@ -618,6 +618,7 @@ leader_election: fn validation_rejects_replica_groups_without_leader_election() { let mut cfg = dev_config(); cfg.replica_groups = 2; + cfg.task_store.backend = "redis".into(); // Must be redis to test leader_election independently cfg.leader_election.enabled = false; let err = cfg.validate().unwrap_err(); assert!(err.to_string().contains("leader_election")); @@ -702,6 +703,8 @@ leader_election: fn validation_accepts_tenant_affinity_with_valid_groups() { let mut cfg = dev_config(); cfg.replica_groups = 3; + cfg.task_store.backend = "redis".into(); // Multi-group requires redis + cfg.leader_election.enabled = true; // Multi-group requires leader election cfg.tenant_affinity.enabled = true; cfg.tenant_affinity.dedicated_groups = vec![0, 1, 2]; cfg.tenant_affinity.static_map.insert("tenant1".into(), 0); diff --git a/crates/miroir-core/src/config/validate.rs b/crates/miroir-core/src/config/validate.rs index 557853d..69da29d 100644 --- a/crates/miroir-core/src/config/validate.rs +++ b/crates/miroir-core/src/config/validate.rs @@ -8,14 +8,6 @@ pub fn validate(cfg: &MiroirConfig) -> Result<(), ConfigError> { )); } - // replica_groups > 1 requires redis backend - if cfg.replica_groups > 1 && cfg.task_store.backend == "sqlite" { - return Err(ConfigError::Validation( - "replica_groups > 1 requires task_store.backend = 'redis' (SQLite is single-writer)" - .into(), - )); - } - // Nodes must belong to a valid replica group if cfg.replica_groups > 0 { for node in &cfg.nodes { @@ -77,11 +69,18 @@ pub fn validate(cfg: &MiroirConfig) -> Result<(), ConfigError> { )); } - // Leader election should be enabled when replica_groups > 1 - if cfg.replica_groups > 1 && !cfg.leader_election.enabled { - return Err(ConfigError::Validation( - "leader_election.enabled must be true when replica_groups > 1".into(), - )); + // replica_groups > 1 requires both redis backend and leader election + if cfg.replica_groups > 1 { + if cfg.task_store.backend != "redis" { + return Err(ConfigError::Validation( + "replica_groups > 1 requires task_store.backend = 'redis' (SQLite is single-writer)".into(), + )); + } + if !cfg.leader_election.enabled { + return Err(ConfigError::Validation( + "leader_election.enabled must be true when replica_groups > 1".into(), + )); + } } // Tenant affinity dedicated_groups must be within valid range