From 67904a6c18ea4ea4e24c3bd3e070a62ddc6ce966 Mon Sep 17 00:00:00 2001 From: cxymds Date: Thu, 30 Jul 2026 11:14:38 +0800 Subject: [PATCH] fix(ecstore): start with unresolved Kubernetes peers (#5460) * fix(ecstore): start with unresolved Kubernetes peers * fix(ecstore): infer Kubernetes endpoint identity safely * fix(ecstore): fail closed on unsafe format migration * fix(ecstore): reject poisoned format heal candidates * fix(ecstore): reject unsafe legacy migration outliers * fix(ecstore): resume interrupted format migrations * fix(ecstore): preserve Kubernetes startup compatibility --- Cargo.lock | 1 + Cargo.toml | 1 + crates/config/README.md | 4 + crates/config/src/constants/app.rs | 4 + crates/ecstore/Cargo.toml | 1 + .../src/cluster/rpc/peer_rest_client.rs | 206 ++- crates/ecstore/src/cluster/rpc/remote_disk.rs | 6 +- crates/ecstore/src/core/sets.rs | 219 ++- crates/ecstore/src/layout/disks_layout.rs | 13 +- crates/ecstore/src/layout/endpoint.rs | 21 +- crates/ecstore/src/layout/endpoints.rs | 1221 +++++++++++++++-- crates/ecstore/src/layout/format.rs | 99 +- crates/ecstore/src/runtime/sources.rs | 25 +- .../ecstore/src/services/notification_sys.rs | 13 +- crates/ecstore/src/services/tier/tier.rs | 74 +- crates/ecstore/src/set_disk/mod.rs | 5 +- crates/ecstore/src/set_disk/ops/heal.rs | 67 +- crates/ecstore/src/set_disk/ops/locking.rs | 123 +- crates/ecstore/src/store/heal.rs | 139 +- crates/ecstore/src/store/init.rs | 18 +- crates/ecstore/src/store/init_format.rs | 1209 ++++++++++++++-- crates/ecstore/src/store/mod.rs | 4 +- crates/utils/src/net.rs | 24 +- crates/utils/src/string.rs | 13 +- docs/architecture/compat-cleanup-register.md | 4 + helm/README.md | 82 +- helm/rustfs/templates/_helpers.tpl | 18 +- helm/rustfs/templates/statefulset.yaml | 96 +- helm/rustfs/values.yaml | 30 +- scripts/test_helm_templates.sh | 339 ++++- 30 files changed, 3618 insertions(+), 461 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b927192e..dd420db90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9128,6 +9128,7 @@ dependencies = [ "google-cloud-storage", "hex-simd", "hmac 0.13.0", + "hostname", "hotpath", "http 1.5.0", "http-body 1.1.0", diff --git a/Cargo.toml b/Cargo.toml index 1be11daf9..d8512078f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -256,6 +256,7 @@ hashbrown = { version = "0.17.1" } hex = "0.4.3" hex-simd = "0.8.0" highway = { version = "1.3.0" } +hostname = "0.4.2" ipnetwork = { version = "0.21.1" } lazy_static = "1.5.0" libc = "0.2.189" diff --git a/crates/config/README.md b/crates/config/README.md index 573153824..92141d4e8 100644 --- a/crates/config/README.md +++ b/crates/config/README.md @@ -66,6 +66,10 @@ Current guidance: - `RUSTFS_BROWSER_REDIRECT_URL` sets the externally reachable browser origin used for OIDC callback, console success redirect, and logout fallback URLs. Configure it to the public scheme and authority without a path, for example `https://console.example.com`. In load-balancer deployments, keep OIDC authorize and callback requests on the same backend node because the in-flight OIDC `state` is local to the RustFS node. +## Distributed endpoint locality + +- `RUSTFS_LOCAL_ENDPOINT_HOST` identifies this server's host in a distributed `RUSTFS_VOLUMES` topology without resolving every peer during startup. Set it to exactly one host, without a scheme, port, or path. It is accepted only for orchestrated URL topologies and must match at least one endpoint on the RustFS server port; invalid or unmatched values fail startup. Leave it unset to retain DNS-based locality discovery. + ## Scanner environment aliases - `RUSTFS_SCANNER_SPEED` (canonical, also accepts `MINIO_SCANNER_SPEED`) diff --git a/crates/config/src/constants/app.rs b/crates/config/src/constants/app.rs index 405bb7345..9a6a06471 100644 --- a/crates/config/src/constants/app.rs +++ b/crates/config/src/constants/app.rs @@ -131,6 +131,10 @@ pub const ENV_RUSTFS_ADDRESS: &str = "RUSTFS_ADDRESS"; /// Environment variable for server volumes. pub const ENV_RUSTFS_VOLUMES: &str = "RUSTFS_VOLUMES"; +/// Environment variable identifying this server's host in distributed endpoint +/// lists without relying on DNS locality discovery. +pub const ENV_LOCAL_ENDPOINT_HOST: &str = "RUSTFS_LOCAL_ENDPOINT_HOST"; + /// Environment variable to explicitly bypass local physical disk independence checks. pub const ENV_UNSAFE_BYPASS_DISK_CHECK: &str = "RUSTFS_UNSAFE_BYPASS_DISK_CHECK"; diff --git a/crates/ecstore/Cargo.toml b/crates/ecstore/Cargo.toml index dbc1d769f..31a5446d1 100644 --- a/crates/ecstore/Cargo.toml +++ b/crates/ecstore/Cargo.toml @@ -105,6 +105,7 @@ tempfile.workspace = true hyper = { workspace = true, features = ["http2", "http1", "server"] } hyper-util = { workspace = true, features = ["tokio", "server-auto", "server-graceful", "tracing"] } hyper-rustls = { workspace = true, default-features = false, features = ["native-tokio", "http1", "tls12", "logging", "http2", "aws-lc-rs"] } +hostname.workspace = true rustls = { workspace = true, default-features = false, features = ["aws-lc-rs", "logging", "tls12", "prefer-post-quantum", "std"] } rustls-pki-types.workspace = true tokio = { workspace = true, features = ["io-util", "sync", "signal", "fs", "rt-multi-thread"] } diff --git a/crates/ecstore/src/cluster/rpc/peer_rest_client.rs b/crates/ecstore/src/cluster/rpc/peer_rest_client.rs index 10d2b4a3e..dc4f0bef6 100644 --- a/crates/ecstore/src/cluster/rpc/peer_rest_client.rs +++ b/crates/ecstore/src/cluster/rpc/peer_rest_client.rs @@ -58,7 +58,7 @@ use std::{ collections::HashMap, io::Cursor, sync::{ - Arc, + Arc, Weak, atomic::{AtomicBool, Ordering}, }, time::SystemTime, @@ -350,6 +350,44 @@ impl PeerRestClient { } } + fn parse_topology_host(peer_host_port: &str, grid_host: &str) -> Result { + let url = url::Url::parse(grid_host).map_err(|_| Error::other("peer grid host is not a valid URL"))?; + if !matches!(url.scheme(), "http" | "https") + || !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + || url.path() != "/" + { + return Err(Error::other("peer grid host has an invalid URL shape")); + } + let url_host = url.host().ok_or_else(|| Error::other("peer grid host is missing a host"))?; + let topology_host = match url.port() { + Some(port) => format!("{url_host}:{port}"), + None => url_host.to_string(), + }; + let explicit_port = url.port(); + let name = match url_host { + url::Host::Domain(domain) => domain.to_string(), + url::Host::Ipv4(address) => address.to_string(), + url::Host::Ipv6(address) if explicit_port.is_none() => format!("[{address}]"), + url::Host::Ipv6(address) => address.to_string(), + }; + let port = url + .port_or_known_default() + .filter(|port| *port > 0) + .ok_or_else(|| Error::other("peer grid host is missing a valid port"))?; + let host = XHost { + name, + port, + is_port_set: explicit_port.is_some(), + }; + if topology_host != peer_host_port { + return Err(Error::other("peer topology host does not match its grid URL")); + } + Ok(host) + } + fn build_clients_from_slots( slots: Vec<(String, Option, bool)>, ) -> (Vec>, Vec>, Vec) { @@ -363,14 +401,14 @@ impl PeerRestClient { } let client = match grid_host { - Some(grid_host) => match XHost::try_from(peer_host_port.clone()) { + Some(grid_host) => match Self::parse_topology_host(&peer_host_port, &grid_host) { Ok(host) => { let mut client = PeerRestClient::new(host, grid_host); client.topology_member = peer_host_port.clone(); Some(client) } Err(err) => { - warn!(peer = %peer_host_port, "Xhost parse failed while constructing peer client: {err:?}"); + warn!(peer = %peer_host_port, "peer topology host parse failed while constructing peer client: {err:?}"); None } }, @@ -519,9 +557,8 @@ impl PeerRestClient { } let grid_host = self.grid_host.clone(); - let offline = Arc::clone(&self.offline); - let recovery_running = Arc::clone(&self.recovery_running); - let span = Self::recovery_monitor_span(&grid_host); + let offline = Arc::downgrade(&self.offline); + let recovery_running = Arc::downgrade(&self.recovery_running); // The offline flag and its recovery are the silent half of // rustfs/backlog#888: log the monitor's start and its success so an // "offline then back" episode leaves a trace on the observing node. @@ -530,13 +567,34 @@ impl PeerRestClient { grid_host = %self.grid_host, "peer RPC connection marked offline after a network-like failure; starting background recovery monitor" ); + drop(Self::spawn_recovery_monitor(grid_host, offline, recovery_running)); + } + + fn spawn_recovery_monitor( + grid_host: String, + offline: Weak, + recovery_running: Weak, + ) -> tokio::task::JoinHandle<()> { + let span = Self::recovery_monitor_span(&grid_host); super::spawn_background_monitor(span, async move { let mut delay = get_drive_active_check_interval(); let connect_timeout = get_drive_active_check_timeout(); for attempt in 1..=PEER_REST_RECOVERY_MAX_ATTEMPTS { + if offline.strong_count() == 0 || recovery_running.strong_count() == 0 { + return; + } tokio::time::sleep(delay).await; + if offline.strong_count() == 0 || recovery_running.strong_count() == 0 { + return; + } if Self::perform_connectivity_check(&grid_host, connect_timeout).await.is_ok() { + let Some(offline) = offline.upgrade() else { + return; + }; + let Some(recovery_running) = recovery_running.upgrade() else { + return; + }; offline.store(false, Ordering::Release); recovery_running.store(false, Ordering::Release); info!( @@ -556,8 +614,10 @@ impl PeerRestClient { attempts = PEER_REST_RECOVERY_MAX_ATTEMPTS, "peer recovery monitor reached max attempts; will retry on next request" ); - recovery_running.store(false, Ordering::Release); - }); + if let Some(recovery_running) = recovery_running.upgrade() { + recovery_running.store(false, Ordering::Release); + } + }) } #[cfg(test)] @@ -1807,9 +1867,13 @@ fn tier_config_reload_status_outcome(status: tonic::Status) -> TierConfigReloadO mod tests { use super::*; use crate::config::com::STORAGE_CLASS_SUB_SYS; + use crate::layout::{disks_layout::DisksLayout, endpoints::SetupType}; + use rustfs_config::{ENV_KUBERNETES_SERVICE_HOST, ENV_LOCAL_ENDPOINT_HOST, ENV_STARTUP_TOPOLOGY_WAIT_MODE}; use serde_json::Value; + use serial_test::serial; use std::io::{self, Write}; use std::sync::{Arc, Mutex}; + use temp_env::async_with_vars; use tracing_subscriber::{Registry, fmt::MakeWriter, layer::SubscriberExt}; #[test] @@ -1927,30 +1991,115 @@ mod tests { fn build_clients_from_slots_preserves_missing_remote_topology_slots() { let slots = vec![ ("127.0.0.1:9000".to_string(), None, true), - ("127.0.0.1:9001".to_string(), Some("http://127.0.0.1:9001".to_string()), false), + ( + "rustfs-1.invalid:9001".to_string(), + Some("http://rustfs-1.invalid:9001".to_string()), + false, + ), + ("rustfs-2.invalid".to_string(), Some("http://rustfs-2.invalid".to_string()), false), ("127.0.0.1:notaport".to_string(), Some("http://127.0.0.1:notaport".to_string()), false), ("127.0.0.1:9003".to_string(), None, false), ]; let (remote, all, remote_topology_hosts) = PeerRestClient::build_clients_from_slots(slots); - assert_eq!(remote.len(), 3, "local node is excluded but remote slots are not compacted away"); - assert_eq!(all.len(), 4, "all slots preserve the sorted cluster topology shape"); + assert_eq!(remote.len(), 4, "local node is excluded but remote slots are not compacted away"); + assert_eq!(all.len(), 5, "all slots preserve the sorted cluster topology shape"); assert_eq!( remote_topology_hosts, vec![ - "127.0.0.1:9001".to_string(), + "rustfs-1.invalid:9001".to_string(), + "rustfs-2.invalid".to_string(), "127.0.0.1:notaport".to_string(), "127.0.0.1:9003".to_string() ] ); - assert!(remote[0].is_some(), "valid remote peer should get a client"); - assert!(remote[1].is_none(), "unparseable remote peer should remain observable as a missing slot"); - assert!(remote[2].is_none(), "missing grid host should remain observable as a missing slot"); + let unresolved = remote[0] + .as_ref() + .expect("temporarily unresolved remote peer should retain a client"); + assert_eq!(unresolved.host.to_string(), "rustfs-1.invalid:9001"); + let default_port = remote[1] + .as_ref() + .expect("temporarily unresolved scheme-default remote peer should retain a client"); + assert_eq!(default_port.host.to_string(), "rustfs-2.invalid"); + assert_eq!(default_port.host.port, 80); + assert!(!default_port.host.is_port_set); + assert!(remote[2].is_none(), "unparseable remote peer should remain observable as a missing slot"); + assert!(remote[3].is_none(), "missing grid host should remain observable as a missing slot"); assert!(all[0].is_none(), "local node is represented by the local server_info row"); assert!(all[1].is_some()); - assert!(all[2].is_none()); + assert!(all[2].is_some()); assert!(all[3].is_none()); + assert!(all[4].is_none()); + } + + #[test] + fn topology_host_parser_preserves_names_and_bracketed_ipv6() { + let domain = PeerRestClient::parse_topology_host("rustfs-1.invalid", "https://rustfs-1.invalid") + .expect("unresolved HTTPS topology host should parse without DNS"); + assert_eq!(domain.to_string(), "rustfs-1.invalid"); + assert_eq!(domain.port, 443); + assert!(!domain.is_port_set); + + let ipv6 = PeerRestClient::parse_topology_host("[2001:db8::1]:9000", "http://[2001:db8::1]:9000") + .expect("bracketed IPv6 topology host should parse without changing its identity"); + assert_eq!(ipv6.to_string(), "[2001:db8::1]:9000"); + + let default_port_ipv6 = PeerRestClient::parse_topology_host("[2001:db8::2]", "http://[2001:db8::2]") + .expect("scheme-default IPv6 topology host should parse without DNS"); + assert_eq!(default_port_ipv6.to_string(), "[2001:db8::2]"); + assert_eq!(default_port_ipv6.port, 80); + assert!(!default_port_ipv6.is_port_set); + + assert!(PeerRestClient::parse_topology_host("peer.invalid:0", "http://peer.invalid:0").is_err()); + assert!(PeerRestClient::parse_topology_host("peer-a.invalid:9000", "http://peer-b.invalid:9000").is_err()); + assert!(PeerRestClient::parse_topology_host("peer.invalid:9000", "http://peer.invalid:9000/unexpected").is_err()); + } + + #[tokio::test] + #[serial] + async fn unresolved_default_port_endpoint_topology_retains_all_peer_clients() { + let volumes = (0..4) + .map(|index| format!("http://rustfs-{index}.invalid:80/data{index}")) + .collect::>(); + let layout = DisksLayout::from_volumes(&volumes).expect("distributed default-port topology should parse"); + + async_with_vars( + [ + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("orchestrated")), + (ENV_LOCAL_ENDPOINT_HOST, Some("rustfs-0.invalid")), + (ENV_KUBERNETES_SERVICE_HOST, None), + ], + async { + let (server_pools, setup_type) = EndpointServerPools::create_server_endpoints("0.0.0.0:80", &layout) + .await + .expect("explicit local identity should avoid peer DNS during endpoint construction"); + assert_eq!(setup_type, SetupType::DistErasure); + + let (remote, all, remote_topology_hosts) = + PeerRestClient::build_clients_from_slots(server_pools.peer_grid_host_slots_sorted()); + assert_eq!(remote.len(), 3); + assert!( + remote.iter().all(Option::is_some), + "unresolved remote peers must retain reconnectable clients" + ); + assert_eq!(all.len(), 4); + assert_eq!(all.iter().filter(|client| client.is_none()).count(), 1); + assert_eq!(remote_topology_hosts.len(), 3); + assert!( + remote_topology_hosts.iter().all(|host| !host.contains(':')), + "scheme-default ports must preserve the legacy topology identity" + ); + assert!( + remote + .iter() + .flatten() + .all(|client| client.host.port == 80 && !client.host.is_port_set), + "scheme-default peers must retain the effective dial port" + ); + }, + ) + .await; } #[test] @@ -2740,6 +2889,31 @@ mod tests { assert!(!client.offline.load(Ordering::Acquire)); } + #[tokio::test(start_paused = true)] + async fn dropped_peer_client_releases_and_stops_its_recovery_monitor() { + let client = test_peer_client(); + client.offline.store(true, Ordering::Release); + client.recovery_running.store(true, Ordering::Release); + let offline = Arc::downgrade(&client.offline); + let recovery_running = Arc::downgrade(&client.recovery_running); + let handle = PeerRestClient::spawn_recovery_monitor(client.grid_host.clone(), offline.clone(), recovery_running.clone()); + let started = tokio::time::Instant::now(); + + drop(client); + + assert!(offline.upgrade().is_none(), "detached recovery must not retain offline state"); + assert!( + recovery_running.upgrade().is_none(), + "detached recovery must not retain its running state" + ); + handle.await.expect("recovery monitor should not panic"); + assert_eq!( + tokio::time::Instant::now(), + started, + "recovery monitor should stop before advancing to its first delayed probe" + ); + } + #[tokio::test] async fn peer_rest_client_finalize_result_keeps_online_for_app_errors_mentioning_unavailable() { // Regression: application error text containing "unavailable" (a diff --git a/crates/ecstore/src/cluster/rpc/remote_disk.rs b/crates/ecstore/src/cluster/rpc/remote_disk.rs index d5090d54f..0d8d507e4 100644 --- a/crates/ecstore/src/cluster/rpc/remote_disk.rs +++ b/crates/ecstore/src/cluster/rpc/remote_disk.rs @@ -4814,9 +4814,11 @@ mod tests { async fn test_remote_disk_endpoints_with_different_schemes() { let test_cases = vec![ ("http://server:9000", "server:9000"), - ("https://secure-server:443", "secure-server"), // Default HTTPS port is omitted + ("http://plain-server:80", "plain-server"), + ("http://plain-server", "plain-server"), + ("https://secure-server:443", "secure-server"), ("http://192.168.1.100:8080", "192.168.1.100:8080"), - ("https://secure-server", "secure-server"), // No port specified + ("https://secure-server", "secure-server"), ]; for (url_str, expected_hostname) in test_cases { diff --git a/crates/ecstore/src/core/sets.rs b/crates/ecstore/src/core/sets.rs index 4d8d9f4e7..7b4ba2a60 100644 --- a/crates/ecstore/src/core/sets.rs +++ b/crates/ecstore/src/core/sets.rs @@ -37,7 +37,9 @@ use crate::{ runtime::instance::{InstanceContext, bootstrap_ctx}, runtime::sources as runtime_sources, set_disk::{PreparedGetObjectMetadata, SetDisks}, - store::init_format::{check_format_erasure_values, get_format_erasure_in_quorum, load_format_erasure_all, save_format_file}, + store::init_format::{ + check_format_erasure_values, load_format_erasure_all, save_format_file, select_format_erasure_in_quorum, + }, }; use futures::{ future::join_all, @@ -947,7 +949,7 @@ impl crate::storage_api_contracts::heal::HealOperations for Sets { #[tracing::instrument(skip(self))] async fn heal_format(&self, dry_run: bool) -> Result<(HealResultItem, Option)> { - let (disks, _) = init_storage_disks_with_errors( + let (disks, init_errs) = init_storage_disks_with_errors( &self.endpoints.endpoints, &DiskOption { cleanup: false, @@ -955,15 +957,36 @@ impl crate::storage_api_contracts::heal::HealOperations for Sets { }, ) .await; - let (formats, errs) = load_format_erasure_all(&disks, true).await; + let (formats, mut errs) = load_format_erasure_all(&disks, true).await; + for (err, init_err) in errs.iter_mut().zip(init_errs) { + if init_err.is_some() { + *err = init_err; + } + } + if errs.iter().any(|err| { + matches!( + err, + Some(DiskError::InconsistentDisk | DiskError::CorruptedFormat | DiskError::CorruptedBackend) + ) + }) { + return Ok((HealResultItem::default(), Some(StorageError::CorruptedFormat))); + } if let Err(err) = check_format_erasure_values(&formats, self.set_drive_count) { info!("failed to check formats erasure values: {}", err); return Ok((HealResultItem::default(), Some(err))); } - let ref_format = match get_format_erasure_in_quorum(&formats) { - Ok(format) => format, + let (ref_format, quorum_members) = match select_format_erasure_in_quorum(&formats, 0) { + Ok((format, members)) if format.shared_identity() == self.format.shared_identity() => (format, members), + Ok(_) => return Ok((HealResultItem::default(), Some(StorageError::CorruptedFormat))), Err(err) => return Ok((HealResultItem::default(), Some(err))), }; + if formats + .iter() + .zip(quorum_members) + .any(|(format, member)| format.is_some() && !member) + { + return Ok((HealResultItem::default(), Some(StorageError::CorruptedFormat))); + } let mut res = HealResultItem { heal_item_type: HealItemType::Metadata.to_string(), detail: "disk-format".to_string(), @@ -985,11 +1008,6 @@ impl crate::storage_api_contracts::heal::HealOperations for Sets { return Ok((res, Some(StorageError::NoHealRequired))); } - // if !self.format.eq(&ref_format) { - // info!("format ({:?}) not eq ref_format ({:?})", self.format, ref_format); - // return Ok((res, Some(Error::new(DiskError::CorruptedFormat)))); - // } - let (new_format_sets, _) = new_heal_format_sets(&ref_format, self.set_count, self.set_drive_count, &formats, &errs); if !dry_run { let mut tmp_new_formats = vec![None; self.set_count * self.set_drive_count]; @@ -1298,7 +1316,7 @@ mod tests { assert_eq!(result, (Some(3), Some(1), Some(0))); } - async fn multipart_listing_test_sets() -> (Vec, Arc) { + async fn two_set_test_sets() -> (Vec, Arc) { let format = FormatV3::new(2, 2); let mut temp_dirs = Vec::new(); let mut all_endpoints = Vec::new(); @@ -1339,8 +1357,8 @@ mod tests { Arc::new(RwLock::new(disks)), 2, 1, - 0, set_index, + 0, endpoints, format.clone(), vec![Arc::new(LocalClient::new()), Arc::new(LocalClient::new())], @@ -1373,11 +1391,114 @@ mod tests { (temp_dirs, sets) } + #[tokio::test] + async fn set_format_heal_accepts_quorum_from_a_nonzero_set() { + let (_temp_dirs, sets) = two_set_test_sets().await; + + let (result, err) = sets.disk_set[1] + .heal_format(false) + .await + .expect("the second erasure set should load its own format quorum"); + + assert!(matches!(err, Some(StorageError::NoHealRequired)), "unexpected heal result: {err:?}"); + assert_eq!(result.disk_count, 2); + assert_eq!(result.set_count, 1); + } + + #[tokio::test] + async fn format_heal_rejects_foreign_majorities_at_set_and_pool_scopes() { + let (_temp_dirs, _canonical_format, sets) = setup_heal_format_sets(2, true).await; + let set_disks = set_level_heal_view(&sets).await; + + let (_, set_err) = set_disks + .heal_format(false) + .await + .expect("set format heal should report a typed mismatch"); + assert!( + matches!(set_err, Some(StorageError::CorruptedFormat)), + "foreign set majority must not replace the cached format: {set_err:?}" + ); + + let (_, pool_err) = sets + .heal_format(false) + .await + .expect("pool format heal should report a typed mismatch"); + assert!( + matches!(pool_err, Some(StorageError::CorruptedFormat)), + "foreign pool majority must not replace the cached format: {pool_err:?}" + ); + } + + #[tokio::test] + async fn pool_format_heal_rejects_a_wrong_slot_minority() { + let (_temp_dirs, canonical_format, sets) = setup_heal_format_sets(3, false).await; + let mut poisoned_format = canonical_format.clone(); + poisoned_format.erasure.this = canonical_format.erasure.sets[0][0]; + replace_heal_test_format(&sets, 2, &poisoned_format).await; + let probe_err = new_disk( + &sets.endpoints.endpoints.as_ref()[2], + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect_err("a wrong-slot local format must fail disk initialization"); + assert_eq!(probe_err, DiskError::InconsistentDisk); + + let (_, pool_err) = sets + .heal_format(false) + .await + .expect("pool format heal should report a typed slot mismatch"); + assert!( + matches!(pool_err, Some(StorageError::CorruptedFormat)), + "a wrong-slot minority must not be reported as no-heal-required: {pool_err:?}" + ); + assert_eq!( + read_heal_test_format(&sets, 2).await, + poisoned_format, + "format heal must not overwrite a wrong-slot disk" + ); + } + + #[tokio::test] + async fn format_heal_rejects_a_foreign_minority_at_set_and_pool_scopes() { + let (_temp_dirs, canonical_format, sets) = setup_heal_format_sets(3, false).await; + let mut poisoned_format = canonical_format.clone(); + poisoned_format.id = Uuid::new_v4(); + poisoned_format.erasure.this = poisoned_format.erasure.sets[0][2]; + replace_heal_test_format(&sets, 2, &poisoned_format).await; + let set_disks = set_level_heal_view(&sets).await; + + let (_, set_err) = set_disks + .heal_format(false) + .await + .expect("set format heal should report a typed identity mismatch"); + assert!( + matches!(set_err, Some(StorageError::CorruptedFormat)), + "a foreign minority must not be reported as no-heal-required: {set_err:?}" + ); + + let (_, pool_err) = sets + .heal_format(false) + .await + .expect("pool format heal should report a typed identity mismatch"); + assert!( + matches!(pool_err, Some(StorageError::CorruptedFormat)), + "a foreign minority must not be reported as no-heal-required: {pool_err:?}" + ); + assert_eq!( + read_heal_test_format(&sets, 2).await, + poisoned_format, + "format heal must not overwrite a foreign disk" + ); + } + #[tokio::test(flavor = "multi_thread")] #[serial] async fn list_multipart_uploads_merges_all_sets_without_pagination_loss() { let _setup_type_guard = SetupTypeGuard::switch_to(SetupType::Erasure).await; - let (_temp_dirs, sets) = multipart_listing_test_sets().await; + let (_temp_dirs, sets) = two_set_test_sets().await; let bucket = format!("multipart-list-{}", Uuid::new_v4().simple()); sets.make_bucket(&bucket, &MakeBucketOptions::default()) .await @@ -1616,11 +1737,15 @@ mod tests { // formatting the first `num_formatted` of them against a shared reference // format and leaving the rest unformatted. Returns the live TempDir handles // (must be kept alive), the reference format, and the assembled `Sets`. - // `disk_set` is intentionally empty: these tests only drive `heal_format` - // with `dry_run == true`, which never touches `disk_set`. - async fn setup_heal_format_sets(num_formatted: usize) -> (Vec, FormatV3, Sets) { + // `disk_set` is intentionally empty: these tests only exercise paths that + // return before pool-level healing delegates into a set. + async fn setup_heal_format_sets(num_formatted: usize, foreign_identity: bool) -> (Vec, FormatV3, Sets) { const SET_DRIVE_COUNT: usize = 3; let ref_format = FormatV3::new(1, SET_DRIVE_COUNT); + let mut stored_format = ref_format.clone(); + if foreign_identity { + stored_format.id = Uuid::new_v4(); + } let mut dirs = Vec::with_capacity(SET_DRIVE_COUNT); let mut endpoints = Vec::with_capacity(SET_DRIVE_COUNT); @@ -1645,8 +1770,8 @@ mod tests { ) .await .expect("disk should be created"); - let mut disk_format = ref_format.clone(); - disk_format.erasure.this = ref_format.erasure.sets[0][i]; + let mut disk_format = stored_format.clone(); + disk_format.erasure.this = stored_format.erasure.sets[0][i]; save_format_file(&Some(disk), &Some(disk_format)) .await .expect("format should be saved"); @@ -1677,6 +1802,60 @@ mod tests { (dirs, ref_format, sets) } + async fn set_level_heal_view(sets: &Sets) -> Arc { + let endpoints = sets.endpoints.endpoints.as_ref().clone(); + let mut disks = Vec::with_capacity(endpoints.len()); + for endpoint in &endpoints { + disks.push(Some( + new_disk( + endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("fresh set-level disk handle should open"), + )); + } + + SetDisks::new( + "test-owner".to_string(), + Arc::new(RwLock::new(disks)), + endpoints.len(), + 1, + 0, + 0, + endpoints, + sets.format.clone(), + Vec::new(), + ) + .await + } + + async fn replace_heal_test_format(sets: &Sets, disk_index: usize, format: &FormatV3) { + let disk = new_disk( + &sets.endpoints.endpoints.as_ref()[disk_index], + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("heal test disk should open"); + save_format_file(&Some(disk.clone()), &Some(format.clone())) + .await + .expect("poisoned test format should be written"); + } + + async fn read_heal_test_format(sets: &Sets, disk_index: usize) -> FormatV3 { + let path = std::path::Path::new(&sets.endpoints.endpoints.as_ref()[disk_index].get_file_path()) + .join(crate::disk::RUSTFS_META_BUCKET) + .join(crate::disk::FORMAT_CONFIG_FILE); + let data = tokio::fs::read(path).await.expect("test format should be readable"); + FormatV3::try_from(data.as_slice()).expect("test format should parse") + } + // Regression for #956 (NoHealRequired path): with every disk already // formatted, `heal_format` reports exactly one drive record per disk // (N = set_count * set_drive_count), each carrying a real endpoint. Before @@ -1685,7 +1864,7 @@ mod tests { #[tokio::test] #[serial] async fn heal_format_no_heal_required_reports_one_record_per_disk() { - let (_dirs, _ref_format, sets) = setup_heal_format_sets(3).await; + let (_dirs, _ref_format, sets) = setup_heal_format_sets(3, false).await; let (res, err) = sets.heal_format(true).await.expect("heal_format should succeed"); // All disks formatted -> NoHealRequired early return, still returns `res`. @@ -1715,7 +1894,7 @@ mod tests { #[serial] async fn heal_format_heal_path_reports_one_record_per_disk_aligned() { // Disks 0 and 1 formatted (quorum), disk 2 unformatted. - let (_dirs, _ref_format, sets) = setup_heal_format_sets(2).await; + let (_dirs, _ref_format, sets) = setup_heal_format_sets(2, false).await; let (res, err) = sets.heal_format(true).await.expect("heal_format should succeed"); // Unformatted disk present -> heal path, not NoHealRequired. diff --git a/crates/ecstore/src/layout/disks_layout.rs b/crates/ecstore/src/layout/disks_layout.rs index 06a74494c..f880bfeac 100644 --- a/crates/ecstore/src/layout/disks_layout.rs +++ b/crates/ecstore/src/layout/disks_layout.rs @@ -203,7 +203,7 @@ fn get_all_sets>(set_drive_count: usize, is_ellipses: bool, args: for args in set_args.iter() { for arg in args { if unique_args.contains(arg) { - return Err(Error::other(format!("Input args {arg} has duplicate ellipses"))); + return Err(Error::other("input arguments contain a duplicate endpoint after ellipsis expansion")); } unique_args.insert(arg); } @@ -924,4 +924,15 @@ mod test { } } } + + #[test] + fn layout_errors_do_not_echo_url_credentials() { + for volumes in [ + vec!["http://:duplicate-secret@server/path", "http://:duplicate-secret@server/path"], + vec!["http://:ellipsis...secret@server/path"], + ] { + let err = DisksLayout::from_volumes(&volumes).unwrap_err(); + assert!(!err.to_string().contains("secret"), "layout error leaked endpoint credentials: {err}"); + } + } } diff --git a/crates/ecstore/src/layout/endpoint.rs b/crates/ecstore/src/layout/endpoint.rs index 6a8e86e3e..5e9ca0c58 100644 --- a/crates/ecstore/src/layout/endpoint.rs +++ b/crates/ecstore/src/layout/endpoint.rs @@ -88,6 +88,7 @@ impl TryFrom<&str> for Endpoint { // - All field should be empty except Host and Path. if !((url.scheme() == "http" || url.scheme() == "https") && url.username().is_empty() + && url.password().is_none() && url.fragment().is_none() && url.query().is_none()) { @@ -366,6 +367,12 @@ mod test { expected_type: None, expected_err: Some(Error::other("invalid URL endpoint format")), }, + TestCase { + arg: "http://:topsecret@server/path", + expected_endpoint: None, + expected_type: None, + expected_err: Some(Error::other("invalid URL endpoint format")), + }, TestCase { arg: "http://:/path", expected_endpoint: None, @@ -505,8 +512,18 @@ mod test { let endpoint = Endpoint::try_from("http://example.com:9000/path").unwrap(); assert_eq!(endpoint.host_port(), "example.com:9000"); - let endpoint_no_port = Endpoint::try_from("https://example.com/path").unwrap(); - assert_eq!(endpoint_no_port.host_port(), "example.com"); + for endpoint in [ + Endpoint::try_from("http://example.com/path").unwrap(), + Endpoint::try_from("http://example.com:80/path").unwrap(), + ] { + assert_eq!(endpoint.host_port(), "example.com"); + } + for endpoint in [ + Endpoint::try_from("https://example.com/path").unwrap(), + Endpoint::try_from("https://example.com:443/path").unwrap(), + ] { + assert_eq!(endpoint.host_port(), "example.com"); + } let file_endpoint = Endpoint::try_from("/tmp/data").unwrap(); assert_eq!(file_endpoint.host_port(), ""); diff --git a/crates/ecstore/src/layout/endpoints.rs b/crates/ecstore/src/layout/endpoints.rs index b8e380270..1962ea77c 100644 --- a/crates/ecstore/src/layout/endpoints.rs +++ b/crates/ecstore/src/layout/endpoints.rs @@ -12,17 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{ - layout::{ - disks_layout::DisksLayout, - endpoint::{Endpoint, EndpointType}, - }, - runtime::sources as runtime_sources, +use crate::layout::{ + disks_layout::DisksLayout, + endpoint::{Endpoint, EndpointType}, }; use rustfs_config::{ DEFAULT_STARTUP_TOPOLOGY_RETRY_MAX_DELAY_SECS, DEFAULT_STARTUP_TOPOLOGY_WAIT_TIMEOUT_SECS, DEFAULT_UNSAFE_BYPASS_DISK_CHECK, - ENV_KUBERNETES_SERVICE_HOST, ENV_MINIO_CI, ENV_STARTUP_TOPOLOGY_RETRY_MAX_DELAY, ENV_STARTUP_TOPOLOGY_WAIT_MODE, - ENV_STARTUP_TOPOLOGY_WAIT_TIMEOUT, ENV_UNSAFE_BYPASS_DISK_CHECK, + ENV_KUBERNETES_SERVICE_HOST, ENV_LOCAL_ENDPOINT_HOST, ENV_MINIO_CI, ENV_STARTUP_TOPOLOGY_RETRY_MAX_DELAY, + ENV_STARTUP_TOPOLOGY_WAIT_MODE, ENV_STARTUP_TOPOLOGY_WAIT_TIMEOUT, ENV_UNSAFE_BYPASS_DISK_CHECK, }; use rustfs_utils::{XHost, check_local_server_addr, get_env_opt_str, get_host_ip, is_local_host}; use std::{ @@ -132,10 +129,10 @@ impl> TryFrom<&[T]> for Endpoints { // Loop through args and adds to endpoint list. for (i, arg) in args.iter().enumerate() { - let endpoint = match Endpoint::try_from(arg.as_ref()) { - Ok(ep) => ep, - Err(e) => return Err(Error::other(format!("'{}': {}", arg.as_ref(), e))), - }; + let endpoint = Endpoint::try_from(arg.as_ref()).map_err(|err| { + let err = std::io::Error::from(err); + Error::new(err.kind(), format!("{err} (endpoint argument #{})", i + 1)) + })?; // All endpoints have to be same type and scheme if applicable. if i == 0 { @@ -213,17 +210,23 @@ impl PoolEndpointList { /// creates a list of endpoints per pool, resolves their relevant /// hostnames and discovers those are local or remote. async fn create_pool_endpoints(server_addr: &str, disks_layout: &DisksLayout) -> Result { - Self::create_pool_endpoints_with(server_addr, disks_layout, None).await + Self::create_pool_endpoints_with(server_addr, disks_layout, None, None).await } /// Same as [`create_pool_endpoints`] but lets tests inject an explicit - /// startup topology convergence policy instead of resolving it from the - /// environment. + /// startup topology convergence policy and local endpoint host instead of + /// resolving them from the environment. async fn create_pool_endpoints_with( server_addr: &str, disks_layout: &DisksLayout, policy_override: Option, + local_endpoint_host: Option<&str>, ) -> Result { + let unsupported_explicit_host = || { + Error::other(format!( + "{ENV_LOCAL_ENDPOINT_HOST} is only supported for distributed URL endpoints in orchestrated mode" + )) + }; if disks_layout.is_empty_layout() { return Err(Error::other("invalid number of endpoints")); } @@ -232,6 +235,10 @@ impl PoolEndpointList { // For single arg, return single drive EC setup. if disks_layout.is_single_drive_layout() { + if local_endpoint_host.is_some() { + return Err(unsupported_explicit_host()); + } + let mut endpoint = Endpoint::try_from(disks_layout.get_single_drive_layout())?; endpoint.update_is_local(server_addr.port())?; @@ -293,7 +300,18 @@ impl PoolEndpointList { .first() .and_then(|eps| eps.as_ref().first()) .is_some_and(|ep| ep.get_type() == EndpointType::Url); - let policy = policy_override.unwrap_or_else(|| StartupTopologyPolicy::resolve(distributed)); + let all_distributed = pool_endpoint_list + .inner + .iter() + .flat_map(|endpoints| endpoints.as_ref()) + .all(|endpoint| endpoint.get_type() == EndpointType::Url); + let policy = match policy_override { + Some(policy) => policy, + None => StartupTopologyPolicy::resolve(distributed, local_endpoint_host.is_some())?, + }; + if local_endpoint_host.is_some() && (!all_distributed || !policy.is_orchestrated()) { + return Err(unsupported_explicit_host()); + } info!( target: "rustfs::ecstore::endpoints", mode = ?policy.mode, @@ -305,14 +323,18 @@ impl PoolEndpointList { let convergence_started = Instant::now(); let dns_retry_deadline = DnsRetryDeadline::new(policy.wait_timeout, policy.retry_max_delay); - pool_endpoint_list - .update_is_local(server_addr.port(), &dns_retry_deadline) - .await?; + if let Some(local_endpoint_host) = local_endpoint_host { + pool_endpoint_list.update_is_local_from_explicit_host(disks_layout, server_addr.port(), local_endpoint_host)?; + } else { + pool_endpoint_list + .update_is_local(server_addr.port(), &dns_retry_deadline) + .await?; - // Collapse divergent local/remote verdicts for the same host:port that - // orchestrated DNS churn can produce during startup, before any check - // relies on the local flag. - normalize_same_host_local_state(pool_endpoint_list.as_mut()); + // Collapse divergent local/remote verdicts for the same host:port that + // orchestrated DNS churn can produce during startup, before any check + // relies on the local flag. + normalize_same_host_local_state(pool_endpoint_list.as_mut()); + } for endpoints in pool_endpoint_list.inner.iter_mut() { // Check whether same path is not used in endpoints of a host on different port. @@ -424,7 +446,9 @@ impl PoolEndpointList { } match ep.url.port() { None => { - let _ = ep.url.set_port(Some(server_addr.port())); + if local_endpoint_host.is_none() { + let _ = ep.url.set_port(Some(server_addr.port())); + } } Some(port) => { // If endpoint is local, but port is different than serverAddrPort, then make it as remote. @@ -498,6 +522,73 @@ impl PoolEndpointList { Ok(()) } + + fn update_is_local_from_explicit_host( + &mut self, + disks_layout: &DisksLayout, + local_port: u16, + raw_local_host: &str, + ) -> Result<()> { + let local_host = parse_explicit_local_endpoint_host(raw_local_host)?; + let mut local_endpoint_found = false; + let mut host_path_ports = HashMap::new(); + debug_assert_eq!(self.inner.len(), disks_layout.pools.len()); + + for (endpoints, pool_layout) in self.inner.iter_mut().zip(&disks_layout.pools) { + debug_assert_eq!(endpoints.as_ref().len(), pool_layout.iter().map(Vec::len).sum::()); + for (endpoint, raw_endpoint) in endpoints.as_mut().iter_mut().zip(pool_layout.iter().flatten()) { + let endpoint_host = match endpoint.url.host().map(|host| host.to_owned()) { + Some(Host::Domain(domain)) => { + let canonical_domain = domain_without_optional_trailing_dot(&domain) + .ok_or_else(|| { + Error::new( + ErrorKind::InvalidInput, + "explicit local endpoint identity requires canonical endpoint hostnames", + ) + })? + .to_string(); + Host::Domain(canonical_domain) + } + Some(host) => host, + None => { + return Err(Error::other( + "explicit local endpoint identity requires every distributed endpoint to have a host", + )); + } + }; + let explicit_port = explicit_url_port(raw_endpoint)?; + let endpoint_port = explicit_port.unwrap_or(local_port); + if explicit_port.is_none() { + let _ = endpoint.url.set_port(Some(local_port)); + } + let path = endpoint.get_file_path(); + endpoint.is_local = endpoint_host == local_host && endpoint_port == local_port; + match host_path_ports.entry((endpoint_host, path)) { + Entry::Occupied(entry) if *entry.get() != endpoint_port => { + return Err(Error::other( + "same path can not be served by different port on the same explicit endpoint host", + )); + } + Entry::Occupied(_) => { + return Err(Error::other("duplicate distributed endpoint after explicit host normalization")); + } + Entry::Vacant(entry) => { + entry.insert(endpoint_port); + } + } + + local_endpoint_found |= endpoint.is_local; + } + } + + if !local_endpoint_found { + return Err(Error::other(format!( + "{ENV_LOCAL_ENDPOINT_HOST} does not match any distributed endpoint on the local server port {local_port}" + ))); + } + + Ok(()) + } } const DNS_RETRY_BASE_DELAY: Duration = Duration::from_millis(500); @@ -672,18 +763,170 @@ fn is_retryable_dns_error(err: &Error) -> bool { return true; } - if matches!(err.raw_os_error(), Some(-3) | Some(-2)) { + if err.raw_os_error().is_some_and(is_retryable_dns_raw_os_error) { return true; } - let message = err.to_string().to_ascii_lowercase(); + let message = err.to_string().trim().to_ascii_lowercase(); // Kubernetes and Docker DNS records can be observed as negative lookups // while headless service records are still propagating during startup. - message.contains("temporary failure in name resolution") - || message.contains("try again") - || message.contains("name or service not known") - || message.contains("no such host") - || message.contains("nodename nor servname provided") + matches!( + message.as_str(), + "temporary failure in name resolution" + | "failed to lookup address information: temporary failure in name resolution" + | "name or service not known" + | "failed to lookup address information: name or service not known" + | "name does not resolve" + | "failed to lookup address information: name does not resolve" + | "no such host" + | "no such host is known." + | "nodename nor servname provided, or not known" + | "failed to lookup address information: nodename nor servname provided, or not known" + ) +} + +fn is_retryable_dns_raw_os_error(code: i32) -> bool { + if matches!(code, -3 | -2 | 11001 | 11002 | 11004) { + return true; + } + + cfg!(any( + target_os = "macos", + target_os = "ios", + target_os = "freebsd", + target_os = "openbsd", + target_os = "netbsd", + target_os = "dragonfly" + )) && matches!(code, 2 | 8) +} + +fn explicit_url_port(raw: &str) -> Result> { + let invalid = || { + Error::new( + ErrorKind::InvalidInput, + "explicit local endpoint identity requires canonical HTTP(S) endpoint URLs", + ) + }; + if raw.trim() != raw || raw.contains('\\') || raw.bytes().any(|byte| byte.is_ascii_control()) { + return Err(invalid()); + } + + let (scheme, remainder) = raw.split_once("://").ok_or_else(&invalid)?; + if !scheme.eq_ignore_ascii_case("http") && !scheme.eq_ignore_ascii_case("https") { + return Err(invalid()); + } + let authority = remainder + .split('/') + .next() + .filter(|authority| !authority.is_empty()) + .ok_or_else(&invalid)?; + if authority.contains('@') { + return Err(invalid()); + } + + let port = if let Some(bracketed) = authority.strip_prefix('[') { + let (_, suffix) = bracketed.split_once(']').ok_or_else(&invalid)?; + if suffix.is_empty() { + return Ok(None); + } + suffix.strip_prefix(':').ok_or_else(&invalid)? + } else if let Some((_, port)) = authority.rsplit_once(':') { + port + } else { + return Ok(None); + }; + + port.parse().map(Some).map_err(|_| invalid()) +} + +fn infer_kubernetes_local_endpoint_host( + disks_layout: &DisksLayout, + local_port: u16, + raw_kernel_hostname: &str, +) -> Result> { + let raw_kernel_hostname = raw_kernel_hostname.trim(); + let Host::Domain(kernel_hostname) = Host::parse(raw_kernel_hostname) + .map_err(|_| Error::new(ErrorKind::InvalidData, "kernel hostname is not a valid DNS name"))? + else { + return Err(Error::new( + ErrorKind::InvalidData, + "kernel hostname must be a DNS name for Kubernetes endpoint inference", + )); + }; + let kernel_hostname = domain_without_optional_trailing_dot(&kernel_hostname) + .filter(|hostname| !hostname.contains('*')) + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "kernel hostname is not a canonical DNS name"))?; + let kernel_hostname_is_fqdn = kernel_hostname.contains('.'); + let mut candidates = HashSet::new(); + + for raw_endpoint in disks_layout.pools.iter().flat_map(|pool| pool.iter().flatten()) { + let endpoint = Endpoint::try_from(raw_endpoint.as_str())?; + let Some(Host::Domain(endpoint_host)) = endpoint.url.host() else { + continue; + }; + let endpoint_host = domain_without_optional_trailing_dot(endpoint_host) + .ok_or_else(|| Error::new(ErrorKind::InvalidInput, "distributed endpoint host is not canonical"))?; + let hostname_matches = if kernel_hostname_is_fqdn { + endpoint_host == kernel_hostname + } else { + endpoint_host.split('.').next() == Some(kernel_hostname) + }; + if !hostname_matches { + continue; + } + + let endpoint_port = explicit_url_port(raw_endpoint)?.unwrap_or(local_port); + if endpoint_port == local_port { + candidates.insert(endpoint_host.to_string()); + } + } + + match candidates.len() { + 0 => Ok(None), + 1 => Ok(candidates.into_iter().next()), + _ => Err(Error::new( + ErrorKind::InvalidInput, + "kernel hostname matches multiple distributed endpoint hosts; set RUSTFS_LOCAL_ENDPOINT_HOST explicitly", + )), + } +} + +fn parse_explicit_local_endpoint_host(raw: &str) -> Result> { + let invalid = || { + Error::other(format!( + "{ENV_LOCAL_ENDPOINT_HOST} must contain exactly one host without a scheme, port, or path" + )) + }; + let raw = raw.trim(); + if raw.is_empty() { + return Err(invalid()); + } + + let host = Host::parse(raw).map_err(|_| invalid())?; + let host = match host { + Host::Domain(domain) => Host::Domain( + domain_without_optional_trailing_dot(&domain) + .ok_or_else(&invalid)? + .to_string(), + ), + host => host, + }; + if matches!(&host, Host::Domain(domain) if domain.contains('*')) + || matches!(&host, Host::Ipv4(ip) if ip.is_unspecified()) + || matches!(&host, Host::Ipv6(ip) if ip.is_unspecified() + || ip.to_ipv4_mapped().is_some_and(|mapped| mapped.is_unspecified())) + { + return Err(Error::other(format!( + "{ENV_LOCAL_ENDPOINT_HOST} must not be a wildcard or unspecified address" + ))); + } + + Ok(host) +} + +fn domain_without_optional_trailing_dot(domain: &str) -> Option<&str> { + let canonical = domain.strip_suffix('.').unwrap_or(domain); + (!canonical.is_empty() && !canonical.ends_with('.')).then_some(canonical) } fn dns_retry_delay(attempt: u32, max_delay: Duration) -> Duration { @@ -743,7 +986,7 @@ pub(crate) struct StartupTopologyPolicy { impl StartupTopologyPolicy { /// Resolves the policy from the environment. `distributed` is true when the /// endpoints are URL style (the only ones that resolve hostnames). - fn resolve(distributed: bool) -> Self { + fn resolve(distributed: bool, explicit_local_host: bool) -> Result { // `KUBERNETES_SERVICE_HOST` is platform-injected, so probe it directly // rather than through the RUSTFS/MINIO config alias helpers. Self::resolve_from( @@ -752,6 +995,7 @@ impl StartupTopologyPolicy { distributed, get_env_opt_str(ENV_STARTUP_TOPOLOGY_WAIT_TIMEOUT).as_deref(), get_env_opt_str(ENV_STARTUP_TOPOLOGY_RETRY_MAX_DELAY).as_deref(), + explicit_local_host, ) } @@ -762,28 +1006,48 @@ impl StartupTopologyPolicy { distributed: bool, timeout_env: Option<&str>, max_delay_env: Option<&str>, - ) -> Self { - let mode = match mode_env.map(|value| value.trim().to_ascii_lowercase()).as_deref() { + explicit_local_host: bool, + ) -> Result { + let mode_env = mode_env.map(|value| value.trim().to_ascii_lowercase()); + let auto_mode = || { + if !distributed { + StartupTopologyWaitMode::FailFast + } else if kubernetes { + StartupTopologyWaitMode::Orchestrated + } else { + StartupTopologyWaitMode::Bounded + } + }; + let mode = match mode_env.as_deref() { Some("orchestrated") => StartupTopologyWaitMode::Orchestrated, Some("bounded") => StartupTopologyWaitMode::Bounded, Some("fail-fast") | Some("failfast") | Some("strict") => StartupTopologyWaitMode::FailFast, - // "auto", unset, or unrecognized: derive from the environment. - // Only URL-style (distributed) endpoints resolve hostnames and need - // to wait for DNS/topology convergence; local path endpoints never - // do, so they fail fast regardless of the platform. - _ => { - if !distributed { - StartupTopologyWaitMode::FailFast - } else if kubernetes { - StartupTopologyWaitMode::Orchestrated - } else { - StartupTopologyWaitMode::Bounded - } + None | Some("") | Some("auto") => auto_mode(), + // RUSTFS_COMPAT_TODO(rustfs-5416-wait-mode): Preserve unknown-mode auto fallback without an anchor. Remove after every supported direct-upgrade chart validates this setting. + Some(_) if !explicit_local_host => auto_mode(), + Some(_) => { + return Err(Error::new( + ErrorKind::InvalidInput, + format!( + "invalid {ENV_STARTUP_TOPOLOGY_WAIT_MODE}; expected auto, orchestrated, bounded, fail-fast, failfast, or strict" + ), + )); } }; - let retry_max_delay = max_delay_env - .and_then(parse_wait_duration) + let parsed_retry_max_delay = max_delay_env.and_then(parse_wait_duration); + if parsed_retry_max_delay.is_some_and(|duration| duration.is_zero()) { + // RUSTFS_COMPAT_TODO(rustfs-5416-zero-retry-delay): Keep zero on the safe default during direct upgrades. Remove after supported configurations no longer rely on the historical parser. + warn!( + event = "startup_topology_zero_retry_delay_defaulted", + component = "ecstore", + subsystem = "endpoint_topology", + state = "compat_default", + "zero startup topology retry delay was replaced with the safe default" + ); + } + let retry_max_delay = parsed_retry_max_delay + .filter(|duration| !duration.is_zero()) .unwrap_or(Duration::from_secs(DEFAULT_STARTUP_TOPOLOGY_RETRY_MAX_DELAY_SECS)); let wait_timeout = match mode { @@ -796,11 +1060,11 @@ impl StartupTopologyPolicy { StartupTopologyWaitMode::FailFast => Duration::ZERO, }; - Self { + Ok(Self { mode, wait_timeout, retry_max_delay, - } + }) } fn is_orchestrated(&self) -> bool { @@ -921,23 +1185,84 @@ impl EndpointServerPools { server_addr: &str, disks_layout: &DisksLayout, ) -> Result<(EndpointServerPools, SetupType)> { - Self::create_server_endpoints_with(server_addr, disks_layout, None).await + let mut local_endpoint_host = get_env_opt_str(ENV_LOCAL_ENDPOINT_HOST); + let mut policy_override = None; + let first_endpoint = disks_layout + .pools + .iter() + .flat_map(|pool| pool.iter().flatten()) + .next() + .map(|endpoint| Endpoint::try_from(endpoint.as_str())) + .transpose()?; + let distributed = first_endpoint + .as_ref() + .is_some_and(|endpoint| endpoint.get_type() == EndpointType::Url); + let mut all_distributed_hosts_are_ip_literals = distributed; + if distributed { + for raw_endpoint in disks_layout.pools.iter().flat_map(|pool| pool.iter().flatten()) { + let endpoint = Endpoint::try_from(raw_endpoint.as_str())?; + if !matches!(endpoint.url.host(), Some(Host::Ipv4(_) | Host::Ipv6(_))) { + all_distributed_hosts_are_ip_literals = false; + break; + } + } + } + let wait_mode = get_env_opt_str(ENV_STARTUP_TOPOLOGY_WAIT_MODE).map(|value| value.trim().to_ascii_lowercase()); + let infer_kubernetes_host = local_endpoint_host.is_none() + && distributed + && !all_distributed_hosts_are_ip_literals + && std::env::var_os(ENV_KUBERNETES_SERVICE_HOST).is_some() + && matches!(wait_mode.as_deref(), None | Some("") | Some("auto") | Some("orchestrated")); + if infer_kubernetes_host { + let kernel_hostname = hostname::get() + .map_err(|err| { + Error::other(format!("failed to read the kernel hostname for Kubernetes endpoint identity: {err}")) + })? + .into_string() + .map_err(|_| Error::new(ErrorKind::InvalidData, "kernel hostname is not valid UTF-8"))?; + let local_port = check_local_server_addr(server_addr)?.port(); + match infer_kubernetes_local_endpoint_host(disks_layout, local_port, &kernel_hostname)? { + Some(inferred_host) => local_endpoint_host = Some(inferred_host), + None => { + // RUSTFS_COMPAT_TODO(rustfs-5416-kubernetes-alias-dns): Keep implicit DNS for pre-anchor Kubernetes aliases. Remove after supported charts always provide an explicit local endpoint host. + if matches!(wait_mode.as_deref(), None | Some("") | Some("auto")) { + policy_override = Some(StartupTopologyPolicy::resolve_from( + Some("bounded"), + true, + distributed, + get_env_opt_str(ENV_STARTUP_TOPOLOGY_WAIT_TIMEOUT).as_deref(), + get_env_opt_str(ENV_STARTUP_TOPOLOGY_RETRY_MAX_DELAY).as_deref(), + false, + )?); + } + warn!( + event = "kubernetes_endpoint_identity_dns_fallback", + component = "ecstore", + subsystem = "endpoint_topology", + state = "legacy_dns", + "kernel hostname did not match a configured endpoint; using compatibility DNS locality" + ); + } + } + } + Self::create_server_endpoints_with(server_addr, disks_layout, policy_override, local_endpoint_host.as_deref()).await } /// Same as [`create_server_endpoints`] but lets tests inject an explicit - /// startup topology convergence policy instead of resolving it from the - /// environment (which would otherwise vary with the CI runner's ambient - /// `KUBERNETES_SERVICE_HOST`). + /// startup topology convergence policy and local endpoint host instead of + /// resolving them from the environment. async fn create_server_endpoints_with( server_addr: &str, disks_layout: &DisksLayout, policy_override: Option, + local_endpoint_host: Option<&str>, ) -> Result<(EndpointServerPools, SetupType)> { if disks_layout.pools.is_empty() { return Err(Error::other("Invalid arguments specified")); } - let pool_eps = PoolEndpointList::create_pool_endpoints_with(server_addr, disks_layout, policy_override).await?; + let pool_eps = + PoolEndpointList::create_pool_endpoints_with(server_addr, disks_layout, policy_override, local_endpoint_host).await?; let mut ret: EndpointServerPools = Vec::with_capacity(pool_eps.as_ref().len()).into(); for (i, eps) in pool_eps.inner.into_iter().enumerate() { @@ -1104,7 +1429,7 @@ impl EndpointServerPools { continue; } let host = endpoint.host_port(); - if endpoint.is_local && endpoint.url.port() == Some(runtime_sources::rustfs_port()) && local.is_none() { + if endpoint.is_local && local.is_none() { local = Some(host.clone()); } @@ -1333,28 +1658,65 @@ mod test { use super::*; - #[cfg(target_os = "linux")] use serial_test::serial; use std::path::Path; - #[cfg(target_os = "linux")] use temp_env::async_with_vars; #[cfg(target_os = "linux")] use tempfile::tempdir; + fn local_flags(endpoints: &Endpoints) -> Vec { + endpoints.as_ref().iter().map(|endpoint| endpoint.is_local).collect() + } + #[test] fn retryable_dns_error_accepts_startup_dns_transients() { assert!(is_retryable_dns_error(&Error::new(ErrorKind::TimedOut, "resolver timeout"))); - assert!(is_retryable_dns_error(&Error::other( - "failed to lookup address information: Name or service not known" - ))); - assert!(is_retryable_dns_error(&Error::other("no such host"))); - assert!(is_retryable_dns_error(&Error::other("nodename nor servname provided, or not known"))); + for message in [ + "temporary failure in name resolution", + "failed to lookup address information: temporary failure in name resolution", + "name or service not known", + "failed to lookup address information: name or service not known", + "name does not resolve", + " Failed to lookup address information: Name does not resolve ", + "no such host", + "no such host is known.", + "nodename nor servname provided, or not known", + "failed to lookup address information: nodename nor servname provided, or not known", + ] { + assert!(is_retryable_dns_error(&Error::other(message)), "{message:?} must be retryable"); + } + assert!(is_retryable_dns_error(&Error::new(ErrorKind::NotFound, "no such host"))); + } + + #[tokio::test] + async fn system_resolver_negative_result_reaches_the_dns_allowlist() { + let err = get_host_ip(Host::Domain("rustfs-startup-negative.invalid")) + .await + .expect_err("the reserved .invalid domain must not resolve"); + assert!( + is_retryable_dns_error(&err), + "system resolver error kind {:?} and message {err:?} must retain retry provenance", + err.kind() + ); } #[test] fn retryable_dns_error_accepts_resolver_raw_os_codes() { - assert!(is_retryable_dns_error(&Error::from_raw_os_error(-3))); - assert!(is_retryable_dns_error(&Error::from_raw_os_error(-2))); + for code in [-3, -2, 11001, 11002, 11004] { + assert!(is_retryable_dns_error(&Error::from_raw_os_error(code))); + } + + let positive_eai = cfg!(any( + target_os = "macos", + target_os = "ios", + target_os = "freebsd", + target_os = "openbsd", + target_os = "netbsd", + target_os = "dragonfly" + )); + for code in [2, 8] { + assert_eq!(is_retryable_dns_error(&Error::from_raw_os_error(code)), positive_eai); + } } #[test] @@ -1364,6 +1726,8 @@ mod test { "invalid URL endpoint format" ))); assert!(!is_retryable_dns_error(&Error::other("mixed scheme is not supported"))); + assert!(!is_retryable_dns_error(&Error::other("request failed: no such host"))); + assert!(!is_retryable_dns_error(&Error::other("try again"))); } #[test] @@ -1409,25 +1773,27 @@ mod test { #[test] fn startup_policy_auto_maps_environment_to_mode() { // Kubernetes -> orchestrated (unbounded by default). - let k8s = StartupTopologyPolicy::resolve_from(None, true, true, None, None); + let k8s = StartupTopologyPolicy::resolve_from(None, true, true, None, None, false).expect("auto mode should resolve"); assert_eq!(k8s.mode, StartupTopologyWaitMode::Orchestrated); assert_eq!(k8s.wait_timeout, Duration::MAX); assert!(k8s.is_orchestrated()); // Distributed URL endpoints, non-Kubernetes -> bounded (default window). - let bounded = StartupTopologyPolicy::resolve_from(None, false, true, None, None); + let bounded = + StartupTopologyPolicy::resolve_from(None, false, true, None, None, false).expect("auto mode should resolve"); assert_eq!(bounded.mode, StartupTopologyWaitMode::Bounded); assert_eq!(bounded.wait_timeout, Duration::from_secs(DEFAULT_STARTUP_TOPOLOGY_WAIT_TIMEOUT_SECS)); assert!(!bounded.is_orchestrated()); // Local path endpoints -> fail-fast (zero wait window). - let local = StartupTopologyPolicy::resolve_from(None, false, false, None, None); + let local = StartupTopologyPolicy::resolve_from(None, false, false, None, None, false).expect("auto mode should resolve"); assert_eq!(local.mode, StartupTopologyWaitMode::FailFast); assert_eq!(local.wait_timeout, Duration::ZERO); // Local path endpoints stay fail-fast even under Kubernetes: they have // no hostnames to resolve, so there is nothing to wait for. - let k8s_local = StartupTopologyPolicy::resolve_from(None, true, false, None, None); + let k8s_local = + StartupTopologyPolicy::resolve_from(None, true, false, None, None, false).expect("auto mode should resolve"); assert_eq!(k8s_local.mode, StartupTopologyWaitMode::FailFast); assert_eq!(k8s_local.wait_timeout, Duration::ZERO); } @@ -1435,30 +1801,64 @@ mod test { #[test] fn startup_policy_explicit_mode_overrides_auto_and_parses_durations() { // Explicit mode wins even under Kubernetes auto-detection. - let forced = StartupTopologyPolicy::resolve_from(Some("bounded"), true, true, Some("2m"), Some("4s")); + let forced = StartupTopologyPolicy::resolve_from(Some("bounded"), true, true, Some("2m"), Some("4s"), false) + .expect("bounded mode should resolve"); assert_eq!(forced.mode, StartupTopologyWaitMode::Bounded); assert_eq!(forced.wait_timeout, Duration::from_secs(120)); assert_eq!(forced.retry_max_delay, Duration::from_secs(4)); // Explicit orchestrated can still be capped by an explicit timeout. - let capped = StartupTopologyPolicy::resolve_from(Some("orchestrated"), false, false, Some("10m"), None); + let capped = StartupTopologyPolicy::resolve_from(Some("orchestrated"), false, false, Some("10m"), None, false) + .expect("orchestrated mode should resolve"); assert_eq!(capped.mode, StartupTopologyWaitMode::Orchestrated); assert_eq!(capped.wait_timeout, Duration::from_secs(600)); - // Unrecognized mode falls back to auto (here: Kubernetes -> orchestrated). - let auto = StartupTopologyPolicy::resolve_from(Some("bogus"), true, true, None, None); - assert_eq!(auto.mode, StartupTopologyWaitMode::Orchestrated); + for auto_value in ["", "auto", " AUTO "] { + let auto = StartupTopologyPolicy::resolve_from(Some(auto_value), true, true, None, None, false) + .expect("explicit auto mode should resolve"); + assert_eq!(auto.mode, StartupTopologyWaitMode::Orchestrated); + } + + let legacy_unknown = StartupTopologyPolicy::resolve_from(Some("invalid-mode"), true, true, None, None, false) + .expect("unknown mode without an explicit host should retain auto compatibility"); + assert_eq!(legacy_unknown.mode, StartupTopologyWaitMode::Orchestrated); + + let invalid = StartupTopologyPolicy::resolve_from(Some("invalid-mode"), true, true, None, None, true).unwrap_err(); + assert_eq!(invalid.kind(), ErrorKind::InvalidInput); + assert!(invalid.to_string().contains(ENV_STARTUP_TOPOLOGY_WAIT_MODE)); // fail-fast accepts a few spellings, trimmed and case-insensitive. for alias in ["fail-fast", "failfast", "strict", " Strict "] { assert_eq!( - StartupTopologyPolicy::resolve_from(Some(alias), false, true, None, None).mode, + StartupTopologyPolicy::resolve_from(Some(alias), false, true, None, None, false) + .expect("fail-fast alias should resolve") + .mode, StartupTopologyWaitMode::FailFast, "alias {alias:?} should resolve to fail-fast" ); } } + #[test] + fn startup_policy_defaults_zero_and_malformed_retry_delays() { + for zero in ["0", "0ms"] { + for explicit_local_host in [false, true] { + let policy = + StartupTopologyPolicy::resolve_from(Some("orchestrated"), true, true, None, Some(zero), explicit_local_host) + .expect("historical zero retry delays should use the safe default"); + assert_eq!(policy.retry_max_delay, Duration::from_secs(DEFAULT_STARTUP_TOPOLOGY_RETRY_MAX_DELAY_SECS)); + } + } + + let malformed = + StartupTopologyPolicy::resolve_from(Some("orchestrated"), true, true, None, Some("invalid-duration"), false) + .expect("malformed historical values should retain default fallback"); + assert_eq!( + malformed.retry_max_delay, + Duration::from_secs(DEFAULT_STARTUP_TOPOLOGY_RETRY_MAX_DELAY_SECS) + ); + } + #[test] fn normalize_same_host_local_state_unifies_divergent_verdicts() { let mut d1 = Endpoint::try_from("http://node1:9000/d1").unwrap(); @@ -1528,12 +1928,8 @@ mod test { ]; let layout = DisksLayout::from_volumes(args.as_slice()).unwrap(); - let bounded = StartupTopologyPolicy { - mode: StartupTopologyWaitMode::Bounded, - wait_timeout: Duration::from_secs(1), - retry_max_delay: DNS_RETRY_MAX_DELAY, - }; - let bounded_err = PoolEndpointList::create_pool_endpoints_with("0.0.0.0:9000", &layout, Some(bounded)) + let bounded = bounded_test_policy(); + let bounded_err = PoolEndpointList::create_pool_endpoints_with("0.0.0.0:9000", &layout, Some(bounded), None) .await .unwrap_err(); assert!( @@ -1543,17 +1939,644 @@ mod test { "bounded mode should run the DNS-IP cross-port check: {bounded_err}" ); - let orchestrated = StartupTopologyPolicy { - mode: StartupTopologyWaitMode::Orchestrated, - wait_timeout: Duration::MAX, - retry_max_delay: DNS_RETRY_MAX_DELAY, - }; - let resolved = PoolEndpointList::create_pool_endpoints_with("0.0.0.0:9000", &layout, Some(orchestrated)) + let orchestrated = orchestrated_test_policy(); + let resolved = PoolEndpointList::create_pool_endpoints_with("0.0.0.0:9000", &layout, Some(orchestrated), None) .await .expect("orchestrated mode should defer the DNS-IP cross-port check"); assert_eq!(resolved.setup_type, SetupType::DistErasure); } + #[test] + fn explicit_local_endpoint_host_accepts_only_a_canonical_host() { + assert_eq!( + parse_explicit_local_endpoint_host(" BÜCHER.example. ").expect("trimmed IDNA host should parse"), + Host::Domain("xn--bcher-kva.example".to_string()) + ); + assert_eq!( + parse_explicit_local_endpoint_host("[2001:db8::1]").expect("bracketed IPv6 host should parse"), + Host::::Ipv6("2001:db8::1".parse().expect("test IPv6 literal should parse")) + ); + assert_eq!( + parse_explicit_local_endpoint_host("192.0.2.10").expect("IPv4 host should parse"), + Host::::Ipv4("192.0.2.10".parse().expect("test IPv4 literal should parse")) + ); + + for invalid in [ + "", + ".", + "example.com..", + "example.com%2e%2e", + "http://example.com", + "example.com:9000", + "example.com/path", + "*", + "*.example.com", + "%2A.example.com", + "0.0.0.0", + "[::]", + "[::ffff:0.0.0.0]", + ] { + assert!( + parse_explicit_local_endpoint_host(invalid).is_err(), + "{invalid:?} must not be accepted as a host-only anchor" + ); + } + } + + #[test] + fn kubernetes_kernel_hostname_inference_requires_one_canonical_host() { + let layout = + DisksLayout::from_volumes(["http://rustfs-{0...2}.rustfs-headless.ns.svc.cluster.local:9000/data"].as_slice()) + .expect("StatefulSet topology should parse"); + for ordinal in 0..3 { + let pod_name = format!("rustfs-{ordinal}"); + let expected_host = format!("{pod_name}.rustfs-headless.ns.svc.cluster.local"); + assert_eq!( + infer_kubernetes_local_endpoint_host(&layout, 9000, &pod_name) + .expect("short kernel hostname should infer safely") + .as_deref(), + Some(expected_host.as_str()) + ); + } + assert_eq!( + infer_kubernetes_local_endpoint_host(&layout, 9000, "rustfs-1.rustfs-headless.ns.svc.cluster.local",) + .expect("FQDN kernel hostname should require an exact match") + .as_deref(), + Some("rustfs-1.rustfs-headless.ns.svc.cluster.local") + ); + assert!( + infer_kubernetes_local_endpoint_host(&layout, 9000, "other-pod") + .expect("no matching endpoint is not ambiguous") + .is_none() + ); + + let wrong_port = DisksLayout::from_volumes(["http://rustfs-1.rustfs-headless.ns.svc.cluster.local:9001/data"].as_slice()) + .expect("wrong-port topology should parse"); + assert!( + infer_kubernetes_local_endpoint_host(&wrong_port, 9000, "rustfs-1") + .expect("a host at another port is not local") + .is_none() + ); + + let ambiguous = DisksLayout::from_volumes( + [ + "http://rustfs-0.first-headless.ns.svc.cluster.local:9000/data0", + "http://rustfs-0.second-headless.ns.svc.cluster.local:9000/data1", + ] + .as_slice(), + ) + .expect("ambiguous topology should parse"); + let err = infer_kubernetes_local_endpoint_host(&ambiguous, 9000, "rustfs-0").unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!(err.to_string().contains(ENV_LOCAL_ENDPOINT_HOST)); + } + + #[test] + fn explicit_url_port_distinguishes_omitted_and_scheme_default_ports() { + assert_eq!(explicit_url_port("http://rustfs-0.example/data").unwrap(), None); + assert_eq!(explicit_url_port("http://rustfs-0.example:80/data").unwrap(), Some(80)); + assert_eq!(explicit_url_port("https://[2001:db8::1]:443/data").unwrap(), Some(443)); + + for invalid in [ + r"http://rustfs-0.example:80\data", + r"http:\\rustfs-0.example:80\data", + r"http:/\rustfs-0.example:80\data", + "http://rustfs-0.example:\t80/data", + "https://[2001:db8::1]:\n443/data", + "http://rustfs-0.example:80 ", + ] { + assert!(explicit_url_port(invalid).is_err(), "{invalid:?} must not bypass raw-port validation"); + } + } + + #[test] + fn endpoint_parse_errors_do_not_echo_url_credentials() { + let err = Endpoints::try_from(["http://:topsecret@server/path"].as_slice()).unwrap_err(); + + assert_eq!(err.to_string(), "invalid URL endpoint format (endpoint argument #1)"); + assert!(!err.to_string().contains("topsecret")); + } + + #[tokio::test] + async fn explicit_local_endpoint_host_selects_only_exact_host_and_server_port() { + let args = vec![ + "http://rustfs-0.rustfs-headless.ns.svc.cluster.local./data0", + "http://rustfs-0.rustfs-headless.ns.svc.cluster.local.:9443/data1", + "http://rustfs-0.rustfs-headless.ns.svc.cluster.local:9001/data2", + "http://localhost:9443/data3", + ]; + let layout = DisksLayout::from_volumes(args.as_slice()).expect("distributed test topology should parse"); + let orchestrated = orchestrated_test_policy(); + + let (server_pools, setup_type) = EndpointServerPools::create_server_endpoints_with( + "0.0.0.0:9443", + &layout, + Some(orchestrated), + Some("RUSTFS-0.RUSTFS-HEADLESS.NS.SVC.CLUSTER.LOCAL"), + ) + .await + .expect("equivalent FQDN spellings should form one local peer identity"); + let local_flags = local_flags(&server_pools.0[0].endpoints); + + assert_eq!(local_flags, vec![true, true, false, false]); + assert_eq!(setup_type, SetupType::DistErasure); + assert_eq!( + server_pools.0[0].endpoints.as_ref()[1].url.host_str(), + Some("rustfs-0.rustfs-headless.ns.svc.cluster.local.") + ); + + let slots = server_pools.peer_grid_host_slots_sorted(); + assert_eq!(slots.len(), 3); + assert_eq!(slots.iter().filter(|(_, _, is_local)| *is_local).count(), 1); + assert!( + slots.iter().all(|(_, grid_host, is_local)| *is_local || grid_host.is_some()), + "canonical host aliases must not create a remote slot without a grid URL" + ); + } + + #[tokio::test] + async fn explicit_ip_endpoint_host_selects_only_the_matching_address() { + let orchestrated = orchestrated_test_policy(); + + for (endpoints, anchor) in [ + ( + [ + "http://192.0.2.10:9443/data0", + "http://192.0.2.11:9443/data1", + "http://192.0.2.12:9443/data2", + "http://192.0.2.13:9443/data3", + ], + "192.0.2.10", + ), + ( + [ + "http://[2001:db8::1]:9443/data0", + "http://[2001:db8::2]:9443/data1", + "http://[2001:db8::3]:9443/data2", + "http://[2001:db8::4]:9443/data3", + ], + "[2001:db8::1]", + ), + ] { + let layout = DisksLayout::from_volumes(endpoints.as_slice()).expect("distributed IP topology should parse"); + let resolved = + PoolEndpointList::create_pool_endpoints_with("0.0.0.0:9443", &layout, Some(orchestrated), Some(anchor)) + .await + .expect("explicit IP anchor should select its exact endpoint"); + let local_flags = local_flags(&resolved.inner[0]); + + assert_eq!(local_flags, vec![true, false, false, false]); + } + } + + #[serial] + #[tokio::test] + async fn create_server_endpoints_reads_explicit_local_host_from_environment() { + assert_eq!(ENV_LOCAL_ENDPOINT_HOST, "RUSTFS_LOCAL_ENDPOINT_HOST"); + async_with_vars( + [ + ("RUSTFS_LOCAL_ENDPOINT_HOST", Some("rustfs-0.rustfs-headless.ns.svc.cluster.local")), + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("orchestrated")), + ], + async { + let endpoints = vec![ + "http://rustfs-0.rustfs-headless.ns.svc.cluster.local:9000/data0".to_string(), + "http://localhost:9000/data1".to_string(), + "http://missing-1.invalid:9000/data2".to_string(), + "http://missing-2.invalid:9000/data3".to_string(), + ]; + + let (pools, setup_type) = EndpointServerPools::from_volumes("0.0.0.0:9000", endpoints) + .await + .expect("production endpoint entry should consume the explicit host environment"); + let local_flags = local_flags(&pools.0[0].endpoints); + + assert_eq!(local_flags, vec![true, false, false, false]); + assert_eq!(setup_type, SetupType::DistErasure); + }, + ) + .await; + } + + #[serial] + #[tokio::test] + async fn create_server_endpoints_infers_kubernetes_pod_host_without_peer_dns() { + let raw_hostname = hostname::get() + .expect("kernel hostname should be available") + .into_string() + .expect("kernel hostname should be UTF-8"); + let Host::Domain(kernel_hostname) = Host::parse(raw_hostname.trim()).expect("kernel hostname should be a DNS name") + else { + panic!("kernel hostname should be a DNS name"); + }; + let kernel_hostname = + domain_without_optional_trailing_dot(&kernel_hostname).expect("kernel hostname should be canonical"); + let local_host = if kernel_hostname.contains('.') { + kernel_hostname.to_string() + } else { + format!("{kernel_hostname}.rustfs-headless.ns.svc.cluster.local") + }; + + async_with_vars( + [ + (ENV_LOCAL_ENDPOINT_HOST, None), + (ENV_KUBERNETES_SERVICE_HOST, Some("10.0.0.1")), + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("auto")), + ], + async { + let (pools, setup_type) = EndpointServerPools::from_volumes( + "0.0.0.0:9000", + vec![ + format!("http://{local_host}:9000/data0"), + "http://permanently-missing.invalid:9000/data1".to_string(), + ], + ) + .await + .expect("kernel hostname inference should avoid resolving the missing peer"); + + assert_eq!(local_flags(&pools.0[0].endpoints), vec![true, false]); + assert_eq!(setup_type, SetupType::DistErasure); + }, + ) + .await; + } + + #[serial] + #[tokio::test] + async fn create_server_endpoints_keeps_ip_literal_kubernetes_topologies() { + async_with_vars( + [ + (ENV_LOCAL_ENDPOINT_HOST, None), + (ENV_KUBERNETES_SERVICE_HOST, Some("10.0.0.1")), + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("auto")), + ], + async { + let (pools, setup_type) = EndpointServerPools::from_volumes( + "127.0.0.1:9000", + vec![ + "http://127.0.0.1:9000/data0".to_string(), + "http://192.0.2.10:9000/data1".to_string(), + ], + ) + .await + .expect("literal IP endpoints should retain DNS-free locality detection"); + + assert_eq!(local_flags(&pools.0[0].endpoints), vec![true, false]); + assert_eq!(setup_type, SetupType::DistErasure); + }, + ) + .await; + } + + #[serial] + #[tokio::test] + async fn create_server_endpoints_bounds_kubernetes_alias_dns_fallback() { + async_with_vars( + [ + (ENV_LOCAL_ENDPOINT_HOST, None), + (ENV_KUBERNETES_SERVICE_HOST, Some("10.0.0.1")), + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("auto")), + (ENV_STARTUP_TOPOLOGY_WAIT_TIMEOUT, Some("0ms")), + ], + async { + let err = EndpointServerPools::from_volumes( + "0.0.0.0:9000", + vec![ + "http://unrelated-0.example.invalid:9000/data0".to_string(), + "http://unrelated-1.example.invalid:9000/data1".to_string(), + ], + ) + .await + .unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::Other); + assert!(err.to_string().contains(TOPOLOGY_TIMEOUT_HINT)); + assert!(!err.to_string().contains(ENV_LOCAL_ENDPOINT_HOST)); + }, + ) + .await; + } + + #[serial] + #[tokio::test] + async fn create_server_endpoints_preserves_resolvable_kubernetes_aliases() { + async_with_vars( + [ + (ENV_LOCAL_ENDPOINT_HOST, None), + (ENV_KUBERNETES_SERVICE_HOST, Some("10.0.0.1")), + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("auto")), + (ENV_STARTUP_TOPOLOGY_WAIT_TIMEOUT, Some("0ms")), + ], + async { + let (pools, setup_type) = EndpointServerPools::from_volumes( + "0.0.0.0:9000", + vec![ + "http://localhost:9000/data0".to_string(), + "http://localhost:9001/data1".to_string(), + ], + ) + .await + .expect("legacy DNS locality should preserve resolvable non-Pod aliases"); + + assert_eq!(local_flags(&pools.0[0].endpoints), vec![true, false]); + assert_eq!(setup_type, SetupType::DistErasure); + }, + ) + .await; + } + + #[serial] + #[tokio::test] + async fn create_server_endpoints_rejects_an_unknown_wait_mode() { + async_with_vars( + [ + ("RUSTFS_LOCAL_ENDPOINT_HOST", Some("rustfs-0.example")), + (ENV_STARTUP_TOPOLOGY_WAIT_MODE, Some("invalid-mode")), + ], + async { + let err = EndpointServerPools::from_volumes( + "0.0.0.0:9000", + vec![ + "http://rustfs-0.example:9000/data0".to_string(), + "http://rustfs-1.example:9000/data1".to_string(), + ], + ) + .await + .unwrap_err(); + + assert_eq!(err.kind(), ErrorKind::InvalidInput); + assert!(err.to_string().contains(ENV_STARTUP_TOPOLOGY_WAIT_MODE)); + }, + ) + .await; + } + + #[tokio::test] + async fn explicit_local_endpoint_host_matches_the_local_pool_in_multi_pool_topology() { + let layout = DisksLayout::from_volumes( + [ + "http://rustfs-{0...1}.rustfs-headless.ns.svc.cluster.local:9000/data", + "http://rustfs-pool1-{0...1}.rustfs-headless.ns.svc.cluster.local:9000/data", + ] + .as_slice(), + ) + .expect("multi-pool distributed topology should parse"); + let orchestrated = orchestrated_test_policy(); + + let resolved = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &layout, + Some(orchestrated), + Some("rustfs-pool1-0.rustfs-headless.ns.svc.cluster.local"), + ) + .await + .expect("multi-pool explicit host should select its local pool"); + let local_endpoints = resolved + .inner + .iter() + .flat_map(|endpoints| endpoints.as_ref()) + .filter(|endpoint| endpoint.is_local) + .collect::>(); + + assert_eq!(local_endpoints.len(), 1); + assert_eq!( + local_endpoints[0].url.host_str(), + Some("rustfs-pool1-0.rustfs-headless.ns.svc.cluster.local") + ); + assert_eq!(local_endpoints[0].pool_idx, 1); + } + + #[tokio::test] + async fn explicit_local_endpoint_host_fails_closed_for_invalid_context_or_zero_match() { + let args = vec![ + "http://192.0.2.10:9000/data0", + "http://192.0.2.11:9000/data1", + "http://192.0.2.12:9000/data2", + "http://192.0.2.13:9000/data3", + ]; + let layout = DisksLayout::from_volumes(args.as_slice()).expect("distributed test topology should parse"); + let orchestrated = orchestrated_test_policy(); + let bounded = bounded_test_policy(); + + let single_drive_layout = + DisksLayout::from_volumes(["/data"].as_slice()).expect("single-drive test topology should parse"); + let single_drive = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &single_drive_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert!( + single_drive + .to_string() + .contains("only supported for distributed URL endpoints in orchestrated mode") + ); + + let zero_match = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &layout, + Some(orchestrated), + Some("rustfs-0.rustfs-headless.ns.svc.cluster.local"), + ) + .await + .unwrap_err(); + assert!(zero_match.to_string().contains("does not match any distributed endpoint")); + + for invalid_host in ["rustfs-0.example..", "rustfs-0.example%2e%2e"] { + let invalid_layout = DisksLayout::from_volumes( + [ + format!("http://{invalid_host}:9000/data0"), + "http://rustfs-1.example:9000/data1".to_string(), + ] + .as_slice(), + ) + .expect("noncanonical hostname reaches explicit identity validation"); + let err = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &invalid_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + } + + for aliases in [ + ["http://remote.example:9000/data", "http://remote.example.:9000/data"], + ["http://remote.example/data", "http://remote.example:9000/data"], + ] { + let alias_layout = + DisksLayout::from_volumes([aliases[0], aliases[1], "http://rustfs-0.example:9000/local-data"].as_slice()) + .expect("raw endpoint aliases are distinct before explicit identity normalization"); + let err = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &alias_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert!( + err.to_string().contains("duplicate distributed endpoint"), + "canonical endpoint aliases must not occupy multiple erasure slots: {err}" + ); + } + + for (scheme, scheme_port, mismatched_local_port) in [("http", 80, 9000), ("https", 443, 9443)] { + let default_port_endpoints = (0..4) + .map(|index| format!("{scheme}://127.0.0.{}:{scheme_port}/data{index}", index + 1)) + .collect::>(); + let default_port_layout = DisksLayout::from_volumes(default_port_endpoints.as_slice()) + .expect("default-port distributed topology should parse"); + + let mismatch = PoolEndpointList::create_pool_endpoints_with( + &format!("0.0.0.0:{mismatched_local_port}"), + &default_port_layout, + Some(orchestrated), + Some("127.0.0.1"), + ) + .await + .unwrap_err(); + assert!( + mismatch.to_string().contains("does not match any distributed endpoint"), + "{scheme} default port must not be replaced by the local server port: {mismatch}" + ); + + let (server_pools, _) = EndpointServerPools::create_server_endpoints_with( + &format!("0.0.0.0:{scheme_port}"), + &default_port_layout, + Some(orchestrated), + Some("127.0.0.1"), + ) + .await + .expect("matching scheme-default topology should build server pools"); + assert!(server_pools.0[0].endpoints.as_ref()[0].is_local); + let slots = server_pools.peer_grid_host_slots_sorted(); + let mut local_slots = 0; + for (peer, grid_host, is_local) in slots { + assert!( + !peer.contains(':'), + "{scheme} peer identity must preserve the legacy default-port spelling: {peer}" + ); + if is_local { + local_slots += 1; + assert!(grid_host.is_none()); + } else { + assert!(grid_host.is_some()); + } + } + assert_eq!(local_slots, 1, "{scheme} default port must retain exactly one local peer slot"); + } + + let noncanonical_layout = + DisksLayout::from_volumes([r"http://rustfs-0.example:80\data0", "http://rustfs-1.example:9000/data1"].as_slice()) + .expect("noncanonical URL reaches endpoint validation"); + let err = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &noncanonical_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::InvalidInput); + + let distinct_host_ports_layout = DisksLayout::from_volumes( + [ + "http://rustfs-0.example:9000/data", + "http://rustfs-1.example:9001/data", + "http://rustfs-2.example:9000/data2", + "http://rustfs-3.example:9000/data3", + ] + .as_slice(), + ) + .expect("distinct-host port topology should parse"); + let distinct_host_ports = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &distinct_host_ports_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .expect("the same path on different hosts and ports must remain valid"); + assert_eq!(distinct_host_ports.setup_type, SetupType::DistErasure); + + let duplicate_path_layout = DisksLayout::from_volumes( + [ + "http://rustfs-0.example:9000/data0", + "http://rustfs-1.example:9000/data1", + "http://rustfs-1.example:9001/data1", + "http://rustfs-2.example:9000/data2", + ] + .as_slice(), + ) + .expect("distributed duplicate-path test topology should parse"); + let duplicate_path = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &duplicate_path_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert!( + duplicate_path + .to_string() + .contains("same path can not be served by different port on the same explicit endpoint host") + ); + + let cross_pool_duplicate_layout = DisksLayout::from_volumes( + [ + "http://rustfs-{0...1}.example:9000/data", + "http://rustfs-{0...1}.example:9001/data", + ] + .as_slice(), + ) + .expect("cross-pool duplicate-path topology should parse"); + let cross_pool_duplicate = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &cross_pool_duplicate_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert!( + cross_pool_duplicate + .to_string() + .contains("same path can not be served by different port on the same explicit endpoint host") + ); + + let non_orchestrated = + PoolEndpointList::create_pool_endpoints_with("0.0.0.0:9000", &layout, Some(bounded), Some("192.0.2.10")) + .await + .unwrap_err(); + assert!( + non_orchestrated + .to_string() + .contains("only supported for distributed URL endpoints in orchestrated mode") + ); + + let mixed_layout = DisksLayout::from_volumes(["http://rustfs-{0...1}.example/data", "/data{0...1}"].as_slice()) + .expect("mixed multi-pool test topology should parse"); + let mixed = PoolEndpointList::create_pool_endpoints_with( + "0.0.0.0:9000", + &mixed_layout, + Some(orchestrated), + Some("rustfs-0.example"), + ) + .await + .unwrap_err(); + assert!( + mixed + .to_string() + .contains("only supported for distributed URL endpoints in orchestrated mode") + ); + } + #[tokio::test] async fn retry_dns_operation_retries_with_backoff_without_real_sleep() { let deadline = DnsRetryDeadline::new(Duration::from_secs(1), DNS_RETRY_MAX_DELAY); @@ -1720,7 +2743,7 @@ mod test { ), ( vec!["ftp://server/d1", "http://server/d2", "http://server/d3", "http://server/d4"], - Some(Error::other("'ftp://server/d1': io error invalid URL endpoint format")), + Some(Error::other("invalid URL endpoint format")), 10, ), ( @@ -1745,7 +2768,7 @@ mod test { "192.168.1.210:9000/tmp/dir2", "192.168.110:9000/tmp/dir3", ], - Some(Error::other("'192.168.1.210:9000/tmp/dir0': io error")), + Some(Error::other("invalid URL endpoint format: missing scheme http or https")), 13, ), ]; @@ -2297,8 +3320,13 @@ mod test { match ( test_case.expected_err, - PoolEndpointList::create_pool_endpoints_with(test_case.server_addr, &disks_layout, Some(bounded_test_policy())) - .await, + PoolEndpointList::create_pool_endpoints_with( + test_case.server_addr, + &disks_layout, + Some(bounded_test_policy()), + None, + ) + .await, ) { (None, Err(err)) => panic!("Test {}: error: expected = , got = {}", test_case.num, err), (Some(err), Ok(_)) => panic!("Test {}: error: expected = {}, got = ", test_case.num, err), @@ -2367,6 +3395,14 @@ mod test { } } + fn orchestrated_test_policy() -> StartupTopologyPolicy { + StartupTopologyPolicy { + mode: StartupTopologyWaitMode::Orchestrated, + wait_timeout: Duration::MAX, + retry_max_delay: DNS_RETRY_MAX_DELAY, + } + } + fn get_expected_endpoints(args: Vec, prefix: String) -> (Vec, Vec) { let mut urls = vec![]; let mut local_flags = vec![]; @@ -2415,7 +3451,8 @@ mod test { }; let ret = - EndpointServerPools::create_server_endpoints_with(test_case.0, &disks_layout, Some(bounded_test_policy())).await; + EndpointServerPools::create_server_endpoints_with(test_case.0, &disks_layout, Some(bounded_test_policy()), None) + .await; if let Err(err) = ret { if test_case.2 { diff --git a/crates/ecstore/src/layout/format.rs b/crates/ecstore/src/layout/format.rs index 592f6f93b..f0a75abf1 100644 --- a/crates/ecstore/src/layout/format.rs +++ b/crates/ecstore/src/layout/format.rs @@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Error as JsonError; use uuid::Uuid; -#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] pub enum FormatMetaVersion { #[serde(rename = "1")] V1, @@ -27,7 +27,7 @@ pub enum FormatMetaVersion { Unknown, } -#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] pub enum FormatBackend { #[serde(rename = "xl")] Erasure, @@ -64,7 +64,7 @@ pub struct FormatErasureV3 { pub distribution_algo: DistributionAlgoVersion, } -#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] pub enum FormatErasureVersion { #[serde(rename = "1")] V1, @@ -77,7 +77,7 @@ pub enum FormatErasureVersion { Unknown, } -#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash)] pub enum DistributionAlgoVersion { #[serde(rename = "CRCMOD")] V1, @@ -121,6 +121,15 @@ pub struct FormatV3 { pub disk_info: Option, } +pub(crate) type SharedFormatIdentity<'a> = ( + &'a FormatMetaVersion, + &'a FormatBackend, + &'a Uuid, + &'a FormatErasureVersion, + &'a [Vec], + &'a DistributionAlgoVersion, +); + impl TryFrom<&[u8]> for FormatV3 { type Error = JsonError; @@ -198,52 +207,24 @@ impl FormatV3 { } pub fn check_other(&self, other: &FormatV3) -> Result<()> { - let mut tmp = other.clone(); - let this = tmp.erasure.this; - tmp.erasure.this = Uuid::nil(); - - if self.erasure.sets.len() != other.erasure.sets.len() { - return Err(Error::other(format!( - "Expected number of sets {}, got {}", - self.erasure.sets.len(), - other.erasure.sets.len() - ))); + if self.shared_identity() != other.shared_identity() { + return Err(Error::other("storage formats do not match")); } - for i in 0..self.erasure.sets.len() { - if self.erasure.sets[i].len() != other.erasure.sets[i].len() { - return Err(Error::other(format!( - "Each set should be of same size, expected {}, got {}", - self.erasure.sets[i].len(), - other.erasure.sets[i].len() - ))); - } + self.find_disk_index_by_disk_id(other.erasure.this).map(|_| ()) + } - for j in 0..self.erasure.sets[i].len() { - if self.erasure.sets[i][j] != other.erasure.sets[i][j] { - return Err(Error::other(format!( - "UUID on positions {}:{} do not match with, expected {:?} got {:?}: (%w)", - i, - j, - self.erasure.sets[i][j].to_string(), - other.erasure.sets[i][j].to_string(), - ))); - } - } - } - - for i in 0..tmp.erasure.sets.len() { - for j in 0..tmp.erasure.sets[i].len() { - if this == tmp.erasure.sets[i][j] { - return Ok(()); - } - } - } - - Err(Error::other(format!( - "DriveID {:?} not found in any drive sets {:?}", - this, other.erasure.sets - ))) + /// Fields that must agree across every disk in one erasure format, + /// excluding the disk-specific `this` UUID and runtime-only `disk_info`. + pub(crate) fn shared_identity(&self) -> SharedFormatIdentity<'_> { + ( + &self.version, + &self.format, + &self.id, + &self.erasure.version, + &self.erasure.sets, + &self.erasure.distribution_algo, + ) } } @@ -437,6 +418,30 @@ mod test { assert!(result.is_ok()); } + #[test] + fn test_check_other_rejects_shared_identity_mismatches() { + type FormatMutation = (&'static str, fn(&mut FormatV3)); + + let format = FormatV3::new(1, 2); + let mutations: [FormatMutation; 5] = [ + ("meta version", |other| other.version = FormatMetaVersion::Unknown), + ("backend", |other| other.format = FormatBackend::ErasureSingle), + ("deployment id", |other| other.id = Uuid::new_v4()), + ("erasure version", |other| other.erasure.version = FormatErasureVersion::V2), + ("distribution algorithm", |other| { + other.erasure.distribution_algo = DistributionAlgoVersion::V2 + }), + ]; + + for (field, mutate) in mutations { + let mut other = format.clone(); + other.erasure.this = format.erasure.sets[0][0]; + mutate(&mut other); + + assert!(format.check_other(&other).is_err(), "{field} mismatch must be rejected"); + } + } + #[test] fn test_check_other_different_set_count() { let format1 = FormatV3::new(2, 4); diff --git a/crates/ecstore/src/runtime/sources.rs b/crates/ecstore/src/runtime/sources.rs index c3f4f4f68..f2df75d2a 100644 --- a/crates/ecstore/src/runtime/sources.rs +++ b/crates/ecstore/src/runtime/sources.rs @@ -424,7 +424,11 @@ pub(crate) async fn record_local_disk_id(instance_ctx: &Arc, di pub(crate) async fn replace_local_disk_id(previous: Option, current: Option, endpoint: String) { let id_map = local_disk_id_map_handle(); let mut disk_id_map = id_map.write().await; - if let Some(previous_id) = previous { + if let Some(previous_id) = previous + && disk_id_map + .get(&previous_id) + .is_some_and(|registered_endpoint| registered_endpoint == &endpoint) + { disk_id_map.remove(&previous_id); } if let Some(current_id) = current { @@ -558,10 +562,14 @@ pub(crate) async fn init_tier_config_mgr(store: Arc) -> Result<()> { #[cfg(test)] mod tests { - use super::{LockRegistry, local_node_name, set_local_node_name}; + use super::{ + LockRegistry, clear_local_disk_id_map_for_test, local_disk_path_by_id, local_node_name, replace_local_disk_id, + set_local_node_name, + }; use crate::disk::endpoint::Endpoint; use rustfs_lock::{LocalClient, LockClient}; use std::{collections::HashMap, sync::Arc}; + use uuid::Uuid; fn url_endpoint(raw: &str) -> Endpoint { Endpoint { @@ -607,4 +615,17 @@ mod tests { assert_eq!(observed, next); } + + #[tokio::test] + #[serial_test::serial] + async fn clearing_a_stale_disk_id_does_not_remove_another_endpoint() { + clear_local_disk_id_map_for_test().await; + let disk_id = Uuid::new_v4(); + replace_local_disk_id(None, Some(disk_id), "endpoint-a".to_string()).await; + + replace_local_disk_id(Some(disk_id), None, "endpoint-b".to_string()).await; + + assert_eq!(local_disk_path_by_id(&disk_id).await, Some("endpoint-a".to_string())); + clear_local_disk_id_map_for_test().await; + } } diff --git a/crates/ecstore/src/services/notification_sys.rs b/crates/ecstore/src/services/notification_sys.rs index 564120055..c690707ad 100644 --- a/crates/ecstore/src/services/notification_sys.rs +++ b/crates/ecstore/src/services/notification_sys.rs @@ -2044,15 +2044,10 @@ fn synthesized_disks(host: &str, endpoints: &EndpointServerPools, state: ItemSta /// Whether `peer_host` refers to the same node as an endpoint whose /// `host_port()` is `ep_host_port`. /// -/// `PeerRestClient::host` is an `XHost`, which resolves names to an address on -/// construction (`hosts_sorted` -> `XHost::try_from` -> `to_socket_addrs`), so -/// `peer_host` is the resolved `IP:port`. An endpoint's `host_port()`, however, -/// is `url.host():port` — still the raw `hostname:port` on hostname-based -/// deployments. A plain string compare therefore misses on hostname clusters, -/// leaving the synthesized/degraded drive list empty and `unknownDisks` at 0 -/// (rustfs/rustfs#4607 follow-up). Compare directly first (fast path / IP -/// deployments), then canonicalize the endpoint side through the same `XHost` -/// resolution and compare again. +/// Current topology clients preserve the endpoint `hostname:port`, so the +/// direct comparison is the normal path. The resolution fallback keeps +/// compatibility with older or manually constructed clients whose `XHost` +/// contains a resolved `IP:port` (rustfs/rustfs#4607 follow-up). fn endpoint_host_matches(peer_host: &str, ep_host_port: &str) -> bool { if peer_host == ep_host_port { return true; diff --git a/crates/ecstore/src/services/tier/tier.rs b/crates/ecstore/src/services/tier/tier.rs index ce0bc97e0..f60999573 100644 --- a/crates/ecstore/src/services/tier/tier.rs +++ b/crates/ecstore/src/services/tier/tier.rs @@ -72,6 +72,7 @@ use crate::{ cluster::rpc::peer_rest_client::{PeerRestClient, PeerTierMutationState}, config::com::{CONFIG_PREFIX, read_config, read_config_with_metadata}, disk::{MIGRATING_META_BUCKET, RUSTFS_META_BUCKET}, + layout::endpoints::EndpointServerPools, object_api::{GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader}, runtime::sources as runtime_sources, set_disk::get_lock_acquire_timeout, @@ -904,14 +905,17 @@ async fn remote_tier_mutation_peers() -> io::Result io::Result>> { + let (peers, _, remote_topology_hosts) = PeerRestClient::new_clients_with_topology(endpoints).await; let peers = peers .into_iter() .flatten() .map(|peer| Arc::new(peer) as Arc) .collect::>(); - ensure_complete_tier_mutation_commit_peer_set(peers.len(), remote_host_count)?; + ensure_complete_tier_mutation_commit_peer_set(peers.len(), remote_topology_hosts.len())?; Ok(peers) } @@ -4307,6 +4311,34 @@ fn tier_config_not_initialized_error(operation: &str) -> std::io::Error { #[cfg(test)] mod tests { use super::*; + use crate::layout::{ + endpoint::Endpoint, + endpoints::{Endpoints, PoolEndpoints, SetupType}, + }; + + struct SetupTypeGuard { + previous: SetupType, + } + + impl SetupTypeGuard { + async fn switch_to(next: SetupType) -> Self { + let previous = runtime_sources::current_setup_type().await; + runtime_sources::set_setup_type(next).await; + Self { previous } + } + } + + impl Drop for SetupTypeGuard { + fn drop(&mut self) { + let previous = self.previous.clone(); + let handle = tokio::runtime::Handle::current(); + tokio::task::block_in_place(|| { + handle.block_on(async move { + runtime_sources::set_setup_type(previous).await; + }); + }); + } + } fn build_s3_tier(name: &str) -> TierConfig { TierConfig { @@ -6354,6 +6386,42 @@ mod tests { assert!(err.to_string().contains("without peer commit clients"), "{err}"); } + #[tokio::test(flavor = "multi_thread")] + #[serial_test::serial] + async fn tier_mutation_peer_composition_preserves_unresolved_topology_slots() { + let mut endpoints = Vec::new(); + for disk_index in 0..4 { + let mut endpoint = Endpoint::try_from(format!("http://rustfs-{disk_index}.invalid:9000/data{disk_index}").as_str()) + .expect("unresolved topology endpoint should parse without DNS"); + endpoint.is_local = disk_index == 0; + endpoint.set_pool_index(0); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_index); + endpoints.push(endpoint); + } + let topology = EndpointServerPools::from(vec![PoolEndpoints { + legacy: false, + set_count: 1, + drives_per_set: 4, + endpoints: Endpoints::from(endpoints), + cmd_line: "unresolved-tier-mutation-topology".to_string(), + platform: "test".to_string(), + }]); + let _setup_type_guard = SetupTypeGuard::switch_to(SetupType::DistErasure).await; + + let peers = remote_tier_mutation_peers_from_topology(topology) + .await + .expect("every unresolved remote topology slot should retain a tier mutation client"); + assert_eq!( + peers.iter().map(|peer| peer.peer_label()).collect::>(), + vec![ + "http://rustfs-1.invalid:9000".to_string(), + "http://rustfs-2.invalid:9000".to_string(), + "http://rustfs-3.invalid:9000".to_string(), + ] + ); + } + #[tokio::test] async fn coordinator_fanout_prepare_failure_aborts_prepared_peers_without_cas() { let manager = TierConfigMgr::new(); diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index 873837acd..d356108d3 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -110,7 +110,10 @@ use crate::{ object_api::{GetObjectReader, ObjectInfo, PutObjReader}, // event::name::EventName, services::event_notification::{EventArgs, send_event}, - store::init_format::{get_format_erasure_in_quorum, load_format_erasure, load_format_erasure_all, save_format_file}, + store::init_format::{ + formats_match_reference_slots, get_format_erasure_in_quorum, load_format_erasure, load_format_erasure_all, + save_format_file, + }, }; use bytes::Bytes; use bytesize::ByteSize; diff --git a/crates/ecstore/src/set_disk/ops/heal.rs b/crates/ecstore/src/set_disk/ops/heal.rs index db9f27c7e..91e8570d0 100644 --- a/crates/ecstore/src/set_disk/ops/heal.rs +++ b/crates/ecstore/src/set_disk/ops/heal.rs @@ -1373,11 +1373,24 @@ impl crate::storage_api_contracts::heal::HealOperations for SetDisks { async fn heal_format(&self, dry_run: bool) -> Result<(HealResultItem, Option)> { let disks = self.disks.read().await.clone(); let (formats, errs) = load_format_erasure_all(&disks, true).await; - let ref_format = match get_format_erasure_in_quorum(&formats) { - Ok(format) => format, + if errs.iter().any(|err| { + matches!( + err, + Some(DiskError::InconsistentDisk | DiskError::CorruptedFormat | DiskError::CorruptedBackend) + ) + }) { + return Ok((HealResultItem::default(), Some(StorageError::CorruptedFormat))); + } + let slot_offset = self + .set_index + .checked_mul(self.set_drive_count) + .ok_or_else(|| Error::other("erasure set slot offset overflow"))?; + let ref_format = match get_format_erasure_in_quorum(&formats, slot_offset) { + Ok(format) if format.shared_identity() == self.format.shared_identity() => format, + Ok(_) => return Ok((HealResultItem::default(), Some(StorageError::CorruptedFormat))), Err(err) => { let can_use_cached_layout = count_errs(&errs, &DiskError::UnformattedDisk) > 0 - && formats.iter().flatten().all(|format| self.format.check_other(format).is_ok()) + && formats_match_reference_slots(&formats, &self.format, slot_offset) && errs .iter() .all(|err| err.is_none() || matches!(err, Some(DiskError::UnformattedDisk))); @@ -1388,6 +1401,9 @@ impl crate::storage_api_contracts::heal::HealOperations for SetDisks { } } }; + if !formats_match_reference_slots(&formats, &ref_format, slot_offset) { + return Ok((HealResultItem::default(), Some(StorageError::CorruptedFormat))); + } let endpoints = crate::layout::endpoints::Endpoints::from(self.set_endpoints.clone()); let before_drives = crate::layout::set_heal::formats_to_drives_info(&endpoints, &formats, &errs); @@ -1543,11 +1559,16 @@ mod heal_result_report_tests { use crate::disk::error::DiskError; use crate::disk::format::FormatV3; use crate::disk::{DiskAPI as _, DiskOption, DiskStore, RUSTFS_META_TMP_BUCKET, ReadOptions, new_disk}; + use crate::error::Error; use crate::object_api::{ObjectOptions, PutObjReader}; use crate::set_disk::ops::object::hermetic_set_disks_support::hermetic_set_disks_isolated; use crate::storage_api_contracts::bucket::{BucketOperations as _, MakeBucketOptions}; + use crate::storage_api_contracts::heal::HealOperations as _; use crate::storage_api_contracts::object::{ObjectIO as _, ObjectOperations as _}; - use crate::{config::storageclass, store::init_format::save_format_file}; + use crate::{ + config::storageclass, + store::init_format::{load_format_erasure, save_format_file}, + }; use rustfs_common::heal_channel::{DriveState, HealOpts, HealScanMode}; use rustfs_filemeta::{BLOCK_SIZE_V2, FileInfo, ObjectPartInfo, TRANSITION_COMPLETE}; use std::sync::Arc; @@ -1863,6 +1884,44 @@ mod heal_result_report_tests { } } + #[tokio::test] + async fn format_heal_cached_layout_rejects_a_disk_from_another_slot() { + let mut _temp_dirs = Vec::new(); + let mut endpoints = Vec::new(); + let mut disks = Vec::new(); + for disk_index in 0..3 { + let (temp_dir, mut endpoint, disk) = real_disk().await; + endpoint.set_pool_index(0); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_index); + _temp_dirs.push(temp_dir); + endpoints.push(endpoint); + disks.push(Some(disk)); + } + let set = set_disks_with(disks.clone(), endpoints, 1).await; + let mut wrong_slot = set.format.clone(); + wrong_slot.erasure.this = set.format.erasure.sets[0][1]; + save_format_file(&disks[0], &Some(wrong_slot)) + .await + .expect("wrong-slot format fixture should be saved"); + let mut correct_slot = set.format.clone(); + correct_slot.erasure.this = set.format.erasure.sets[0][2]; + save_format_file(&disks[2], &Some(correct_slot)) + .await + .expect("correct format fixture should be saved"); + + let (_, heal_err) = set + .heal_format(false) + .await + .expect("format heal should report the quorum failure in its result"); + + assert!(matches!(heal_err, Some(Error::CorruptedFormat))); + let unformatted = load_format_erasure(disks[1].as_ref().expect("second disk should be online"), true) + .await + .expect_err("a rejected fallback must not format the missing slot"); + assert_eq!(unformatted, DiskError::UnformattedDisk); + } + // Regression for #955: an offline disk must contribute exactly one drive // record. Before the fix the offline branch fell through and pushed a second // (Corrupt) record for the same disk, so `before/after.drives` grew to diff --git a/crates/ecstore/src/set_disk/ops/locking.rs b/crates/ecstore/src/set_disk/ops/locking.rs index a9787ba13..6d842633b 100644 --- a/crates/ecstore/src/set_disk/ops/locking.rs +++ b/crates/ecstore/src/set_disk/ops/locking.rs @@ -343,20 +343,23 @@ impl SetDisks { } }; - // The drive's format may place it in a different erasure set than this - // one. Claiming a misplaced drive into `self.disks` would let two sets - // manage the same drive and degrade together, so reject it here - // (backlog#799 B19). - if set_idx != self.set_index { + // Claiming a misplaced drive into `self.disks` would let two slots or + // sets manage the same drive and degrade together (backlog#799 B19). + if set_idx != self.set_index || self.set_endpoints.get(disk_idx) != Some(ep) { warn!( - "renew_disk: drive {:?} belongs to set {} but is being renewed on set {}; skipping", - ep, set_idx, self.set_index + endpoint = %ep, + format_set_index = set_idx, + format_disk_index = disk_idx, + endpoint_pool_index = ep.pool_idx, + endpoint_set_index = ep.set_idx, + endpoint_disk_index = ep.disk_idx, + expected_pool_index = self.pool_index, + expected_set_index = self.set_index, + "renew_disk rejected a drive whose endpoint and format do not identify the same topology slot" ); return; } - // Check that the endpoint matches - let _ = new_disk.set_disk_id(Some(fm.erasure.this)).await; new_disk.enable_health_check(); @@ -715,6 +718,108 @@ mod tests { drop(temp_dirs); } + #[tokio::test] + async fn renew_disk_rejects_a_format_from_another_slot_or_cluster() { + let disk_count = 3; + let format = FormatV3::new(1, disk_count); + let mut temp_dirs = Vec::with_capacity(disk_count); + let mut endpoints = Vec::with_capacity(disk_count); + let mut fixture_disks = Vec::with_capacity(disk_count); + + for disk_idx in 0..disk_count { + let (temp_dir, endpoint, disk) = make_formatted_local_disk(disk_idx, &format).await; + temp_dirs.push(temp_dir); + endpoints.push(endpoint); + fixture_disks.push(disk); + } + + let set_disks = SetDisks::new( + "test-owner".to_string(), + Arc::new(RwLock::new(vec![Some(fixture_disks[0].clone()), None, None])), + disk_count, + disk_count / 2, + 0, + 0, + endpoints.clone(), + format.clone(), + Vec::new(), + ) + .await; + + let mut other_cluster_format = format.clone(); + other_cluster_format.id = Uuid::new_v4(); + other_cluster_format.erasure.this = format.erasure.sets[0][2]; + save_format_file(&Some(fixture_disks[2].clone()), &Some(other_cluster_format)) + .await + .expect("other-cluster format should be written for the rejection test"); + + set_disks.renew_disk(&endpoints[2]).await; + + let disks = set_disks.get_disks_internal().await; + assert_eq!( + disks[0] + .as_ref() + .expect("the canonical first slot must remain attached") + .endpoint(), + endpoints[0] + ); + assert!( + disks[2].is_none(), + "a disk from another deployment must remain detached even when its slot UUID matches" + ); + + let mut correct_format = format.clone(); + correct_format.erasure.this = format.erasure.sets[0][2]; + let replacement_disk = new_disk( + &endpoints[2], + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("third endpoint should reopen after other-cluster rejection"); + save_format_file(&Some(replacement_disk), &Some(correct_format)) + .await + .expect("correct slot format should be restored"); + + set_disks.renew_disk(&endpoints[2]).await; + + let disks = set_disks.get_disks_internal().await; + assert_eq!( + disks[0] + .as_ref() + .expect("the canonical first slot must remain attached") + .endpoint(), + endpoints[0] + ); + assert_eq!(disks[2].as_ref().expect("the restored third slot should attach").endpoint(), endpoints[2]); + + let third_disk = disks[2].clone(); + let mut wrong_slot_format = format.clone(); + wrong_slot_format.erasure.this = format.erasure.sets[0][0]; + save_format_file(&third_disk, &Some(wrong_slot_format)) + .await + .expect("wrong-slot format should be written for the rejection test"); + set_disks.disks.write().await[2] = None; + + let mut misplaced_endpoint = endpoints[2].clone(); + misplaced_endpoint.set_disk_index(0); + set_disks.renew_disk(&misplaced_endpoint).await; + + let disks = set_disks.get_disks_internal().await; + assert_eq!( + disks[0] + .as_ref() + .expect("the canonical first slot must remain attached") + .endpoint(), + endpoints[0] + ); + assert!(disks[2].is_none(), "a disk claiming another endpoint's slot must remain detached"); + + drop(temp_dirs); + } + // SetDisks split P0 (#816): the borrow handle must mirror the core state and // the List operation family must run identically through it. #[tokio::test] diff --git a/crates/ecstore/src/store/heal.rs b/crates/ecstore/src/store/heal.rs index 26190dd85..fe99c1291 100644 --- a/crates/ecstore/src/store/heal.rs +++ b/crates/ecstore/src/store/heal.rs @@ -30,6 +30,7 @@ impl ECStore { }; let mut count_no_heal = 0; + let mut first_error = None; for pool in self.pools.iter() { let (mut result, err) = pool.heal_format(dry_run).await?; if let Some(err) = err { @@ -37,8 +38,8 @@ impl ECStore { StorageError::NoHealRequired => { count_no_heal += 1; } - _ => { - continue; + err => { + first_error.get_or_insert(err); } } } @@ -47,6 +48,9 @@ impl ECStore { r.before.drives.append(&mut result.before.drives); r.after.drives.append(&mut result.after.drives); } + if let Some(err) = first_error { + return Ok((r, Some(err))); + } if count_no_heal == self.pools.len() { info!( event = EVENT_HEAL_FORMAT_COMPLETED, @@ -165,3 +169,134 @@ impl ECStore { Err(StorageError::NotImplemented) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::disk::{DiskOption, format::FormatV3, new_disk}; + use crate::layout::endpoints::{Endpoints, PoolEndpoints}; + use crate::store::init_format::{load_format_erasure, save_format_file}; + + #[tokio::test] + async fn handle_heal_format_continues_after_a_pool_error() { + let canonical_format = FormatV3::new(1, 3); + let mut foreign_format = canonical_format.clone(); + foreign_format.id = Uuid::new_v4(); + let mut temp_dirs = Vec::new(); + let mut endpoints = Vec::new(); + let mut disks = Vec::new(); + + for disk_index in 0..3 { + let temp_dir = tempfile::tempdir().expect("temporary disk root should be created"); + let mut endpoint = Endpoint::try_from(temp_dir.path().to_str().expect("temporary path should be UTF-8")) + .expect("temporary endpoint should parse"); + endpoint.set_pool_index(0); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_index); + let disk = new_disk( + &endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("temporary disk should open"); + let mut disk_format = foreign_format.clone(); + disk_format.erasure.this = foreign_format.erasure.sets[0][disk_index]; + save_format_file(&Some(disk.clone()), &Some(disk_format)) + .await + .expect("foreign format should be written"); + temp_dirs.push(temp_dir); + endpoints.push(endpoint); + disks.push(Some(disk)); + } + + let pool_endpoints = PoolEndpoints { + legacy: false, + set_count: 1, + drives_per_set: 3, + endpoints: Endpoints::from(endpoints), + cmd_line: "foreign-format-majority-test".to_string(), + platform: "test".to_string(), + }; + let pool = Sets::new(disks, &pool_endpoints, &canonical_format, 0, 1) + .await + .expect("test pool should build around the cached canonical format"); + + let mut recoverable_format = FormatV3::new(1, 3); + recoverable_format.id = canonical_format.id; + let mut recoverable_temp_dirs = Vec::new(); + let mut recoverable_endpoints = Vec::new(); + let mut recoverable_disks = Vec::new(); + let mut unformatted_disk = None; + for disk_index in 0..3 { + let temp_dir = tempfile::tempdir().expect("temporary disk root should be created"); + let mut endpoint = Endpoint::try_from(temp_dir.path().to_str().expect("temporary path should be UTF-8")) + .expect("temporary endpoint should parse"); + endpoint.set_pool_index(1); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_index); + let disk = new_disk( + &endpoint, + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await + .expect("temporary disk should open"); + if disk_index < 2 { + let mut disk_format = recoverable_format.clone(); + disk_format.erasure.this = recoverable_format.erasure.sets[0][disk_index]; + save_format_file(&Some(disk.clone()), &Some(disk_format)) + .await + .expect("recoverable format should be written"); + } else { + unformatted_disk = Some(disk.clone()); + } + recoverable_temp_dirs.push(temp_dir); + recoverable_endpoints.push(endpoint); + recoverable_disks.push(Some(disk)); + } + let recoverable_pool_endpoints = PoolEndpoints { + legacy: false, + set_count: 1, + drives_per_set: 3, + endpoints: Endpoints::from(recoverable_endpoints), + cmd_line: "recoverable-format-test".to_string(), + platform: "test".to_string(), + }; + let recoverable_pool = Sets::new(recoverable_disks, &recoverable_pool_endpoints, &recoverable_format, 1, 1) + .await + .expect("recoverable test pool should build"); + + let endpoint_pools = EndpointServerPools::from(vec![pool_endpoints.clone(), recoverable_pool_endpoints.clone()]); + let store = ECStore { + id: canonical_format.id, + disk_map: HashMap::new(), + pools: vec![pool, recoverable_pool], + peer_sys: S3PeerSys::new(&endpoint_pools), + pool_meta: RwLock::new(PoolMeta::default()), + rebalance_meta: RwLock::new(None), + decommission_cancelers: RwLock::new(Vec::new()), + start_gate: Mutex::new(()), + pool_meta_save_gate: Mutex::new(()), + ctx: crate::runtime::instance::bootstrap_ctx(), + }; + + let (result, err) = store + .handle_heal_format(false) + .await + .expect("format heal should return the typed pool error"); + assert!( + matches!(err, Some(StorageError::CorruptedFormat)), + "foreign format majority must not be downgraded to a successful heal: {err:?}" + ); + assert_eq!(result.disk_count, 3, "the recoverable pool should still be inspected"); + let healed = load_format_erasure(&unformatted_disk.expect("the unformatted disk handle should be retained"), true) + .await + .expect("the later pool should be healed despite the first pool error"); + assert_eq!(healed.erasure.this, recoverable_format.erasure.sets[0][2]); + } +} diff --git a/crates/ecstore/src/store/init.rs b/crates/ecstore/src/store/init.rs index 4de6f3f45..4e7128188 100644 --- a/crates/ecstore/src/store/init.rs +++ b/crates/ecstore/src/store/init.rs @@ -101,6 +101,10 @@ fn should_retry_local_decommission_resume(err: &Error, attempt: usize) -> bool { matches!(err, Error::ConfigNotFound) && attempt < LOCAL_DECOMMISSION_RESUME_MAX_CONFIG_RETRIES } +fn should_retry_format_load(err: &Error) -> bool { + !matches!(err, Error::CorruptedFormat) +} + fn should_auto_start_rebalance_after_init(decommission_running: bool, rebalance_meta_loaded: bool) -> bool { rebalance_meta_loaded && !decommission_running } @@ -294,7 +298,7 @@ impl ECStore { // periodic monitoring until format loading succeeds. Startup RPC // failures can still spawn recovery probes for peers that come up // after this node. - let (disks, errs) = init_format::init_disks( + let (mut disks, errs) = init_format::init_disks( &pool_eps.endpoints, &DiskOption { cleanup: true, @@ -311,7 +315,7 @@ impl ECStore { loop { match init_format::connect_load_init_formats( pool_first_is_local, - &disks, + &mut disks, pool_eps.set_count, pool_eps.drives_per_set, deployment_id, @@ -319,6 +323,7 @@ impl ECStore { .await { Ok(fm) => break Ok(fm), + Err(e) if !should_retry_format_load(&e) => break Err(e), // Wrap the final error if we are giving up Err(e) if times >= 10 => { break Err(Error::other(format!("store init failed to load formats after {times} retries: {e}"))); @@ -551,7 +556,7 @@ mod tests { LOCAL_DECOMMISSION_RESUME_MAX_CONFIG_RETRIES, load_pool_meta_for_startup, pool_first_endpoint_is_local, pool_meta_has_active_decommission, preflight_startup_rpc_secret_with, resolve_startup_pool_defaults_with, resolve_store_init_stage_result, save_validated_pool_meta_for_startup, should_auto_start_rebalance_after_init, - should_retry_local_decommission_resume, wait_for_local_decommission_resume_delay, + should_retry_format_load, should_retry_local_decommission_resume, wait_for_local_decommission_resume_delay, }; #[cfg(feature = "test-util")] use crate::{ @@ -773,6 +778,13 @@ mod tests { assert!(!should_retry_local_decommission_resume(&StorageError::SlowDown, 0)); } + #[test] + fn test_should_retry_format_load_rejects_permanent_corruption() { + assert!(!should_retry_format_load(&StorageError::CorruptedFormat)); + assert!(should_retry_format_load(&StorageError::ErasureReadQuorum)); + assert!(should_retry_format_load(&StorageError::FirstDiskWait)); + } + #[test] fn test_should_auto_start_rebalance_after_init_allows_loaded_rebalance_without_decommission() { assert!(should_auto_start_rebalance_after_init(false, true)); diff --git a/crates/ecstore/src/store/init_format.rs b/crates/ecstore/src/store/init_format.rs index 01ecb84b6..bd4ccb301 100644 --- a/crates/ecstore/src/store/init_format.rs +++ b/crates/ecstore/src/store/init_format.rs @@ -20,16 +20,21 @@ use crate::{ disk::{ DiskInfoOptions, DiskOption, DiskStore, FORMAT_CONFIG_FILE, MIGRATING_META_BUCKET, RUSTFS_META_BUCKET, error::DiskError, - format::{FormatErasureVersion, FormatMetaVersion, FormatV3}, + format::{FormatBackend, FormatErasureVersion, FormatMetaVersion, FormatV3}, new_disk, }, layout::endpoints::Endpoints, }; -use futures::future::join_all; -use std::collections::{HashMap, hash_map::Entry}; +use futures::{future::join_all, stream, stream::StreamExt}; +use std::collections::{HashMap, HashSet}; +use tokio::io::AsyncReadExt; use tracing::{debug, error, info, warn}; use uuid::Uuid; +const LEGACY_FORMAT_READ_CONCURRENCY: usize = 16; +const LEGACY_FORMAT_BASE_MAX_BYTES: usize = 16 * 1024; +const LEGACY_FORMAT_MAX_BYTES_PER_DISK: usize = 64; + pub async fn init_disks(eps: &Endpoints, opt: &DiskOption) -> (Vec>, Vec>) { let mut futures = Vec::with_capacity(eps.as_ref().len()); @@ -59,7 +64,7 @@ pub async fn init_disks(eps: &Endpoints, opt: &DiskOption) -> (Vec], + disks: &mut [Option], set_count: usize, set_drive_count: usize, deployment_id: Option, @@ -68,35 +73,51 @@ pub async fn connect_load_init_formats( check_disk_fatal_errs(&errs)?; - check_format_erasure_values(&formats, set_drive_count)?; + let all_unformatted = should_init_erasure_disks(&errs); + let formats_present = formats.iter().flatten().count(); + let mut format_quorum = (formats_present > 0).then(|| select_format_erasure_in_quorum(&formats, 0)); + if format_quorum.as_ref().is_none_or(Result::is_err) + && errs.iter().any(|error| { + matches!( + error, + Some(DiskError::InconsistentDisk | DiskError::CorruptedFormat | DiskError::CorruptedBackend) + ) + }) + { + return Err(Error::CorruptedFormat); + } + let resumable_partial_migration = formats_present > 0 + && errs.iter().any(|error| matches!(error, Some(DiskError::UnformattedDisk))) + && format_quorum.as_ref().is_some_and(Result::is_err) + && formats.iter().zip(&errs).all(|(format, error)| { + format.is_some() || matches!(error, Some(DiskError::UnformattedDisk | DiskError::DiskNotFound)) + }); - if first_disk && should_init_erasure_disks(&errs) { - // UnformattedDisk, try migrate from MinIO format first, else create new format - info!("first_disk && should_init_erasure_disks"); - match try_migrate_format(disks, set_count, set_drive_count).await { - Ok(LegacyFormatOutcome::Migrated(fm)) => { + if first_disk && (all_unformatted || resumable_partial_migration) { + info!(all_unformatted, resumable_partial_migration, "checking for a legacy storage format"); + match try_migrate_format(disks, &formats, set_count, set_drive_count).await { + Ok(LegacyFormatOutcome::Migrated { format, quorum_members }) => { info!("Migrated format from MinIO config"); - return Ok(*fm); + retain_format_quorum_members(disks, &format, &quorum_members, set_drive_count).await?; + return Ok(*format); } Ok(LegacyFormatOutcome::Incompatible) => { - // A MinIO format.json was found on disk but could not be migrated - // (topology/version mismatch or parse failure). Falling through to - // create a FRESH RustFS format changes the object placement layout, - // so the pre-existing MinIO objects will not be readable. Surface - // this loudly instead of silently discarding the legacy data. error!( - "Detected MinIO format.json on disk but could NOT migrate it; initializing a fresh RustFS format instead. \ - Existing MinIO objects will not be readable under the new format. Ensure the RustFS pool / erasure-set \ - topology exactly matches the original MinIO deployment, and that no stale .rustfs.sys/format.json remains." + event = "legacy_format_migration_rejected", + component = "ecstore", + subsystem = "store_init", + state = "incompatible", + "detected MinIO format.json but could not migrate it safely" ); + return Err(Error::CorruptedFormat); } Ok(LegacyFormatOutcome::None) => {} - Err(e) => { - warn!("MinIO format migration attempt failed, will initialize a fresh format: {e}"); - } + Err(e) => return Err(e), + } + if all_unformatted { + let fm = init_format_erasure(disks, set_count, set_drive_count, deployment_id).await?; + return Ok(fm); } - let fm = init_format_erasure(disks, set_count, set_drive_count, deployment_id).await?; - return Ok(fm); } info!( @@ -114,11 +135,53 @@ pub async fn connect_load_init_formats( return Err(Error::FirstDiskWait); } - let fm = get_format_erasure_in_quorum(&formats)?; + let (fm, quorum_members) = match format_quorum.take() { + Some(result) => result?, + None => select_format_erasure_in_quorum(&formats, 0)?, + }; + check_format_erasure_value_for_topology(&fm, formats.len(), set_drive_count)?; + retain_format_quorum_members(disks, &fm, &quorum_members, set_drive_count).await?; Ok(fm) } +async fn retain_format_quorum_members( + disks: &mut [Option], + format: &FormatV3, + quorum_members: &[bool], + set_drive_count: usize, +) -> Result<()> { + if set_drive_count == 0 || quorum_members.len() != disks.len() { + return Err(Error::CorruptedFormat); + } + for (disk, belongs_to_quorum) in disks.iter().zip(quorum_members) { + if !belongs_to_quorum && let Some(disk) = disk { + disk.set_disk_id(None).await?; + } + } + for (index, (disk, belongs_to_quorum)) in disks.iter().zip(quorum_members).enumerate() { + if !belongs_to_quorum { + continue; + } + let disk_id = format + .erasure + .sets + .get(index / set_drive_count) + .and_then(|set| set.get(index % set_drive_count)) + .ok_or(Error::CorruptedFormat)?; + disk.as_ref() + .ok_or(Error::CorruptedFormat)? + .set_disk_id(Some(*disk_id)) + .await?; + } + for (disk, belongs_to_quorum) in disks.iter_mut().zip(quorum_members) { + if !belongs_to_quorum { + *disk = None; + } + } + Ok(()) +} + pub fn quorum_unformatted_disks(errs: &[Option]) -> bool { count_errs(errs, &DiskError::UnformattedDisk) > (errs.len() / 2) } @@ -166,14 +229,17 @@ async fn init_format_erasure( save_format_file_all(disks, &fms).await?; - get_format_erasure_in_quorum(&fms) + get_format_erasure_in_quorum(&fms, 0) } /// Outcome of attempting to migrate an on-disk MinIO `format.json`. enum LegacyFormatOutcome { /// A compatible MinIO format was found and migrated into RustFS format files. /// Boxed to keep the enum small (`FormatV3` is large; the others are unit variants). - Migrated(Box), + Migrated { + format: Box, + quorum_members: Vec, + }, /// A MinIO `format.json` was present but could not be migrated (topology / /// version mismatch, or a parse failure). The caller must decide how to /// proceed; creating a fresh format would leave the legacy objects unreadable. @@ -185,112 +251,261 @@ enum LegacyFormatOutcome { /// Tries to migrate an on-disk MinIO `format.json` into RustFS format files. /// /// Returns [`LegacyFormatOutcome`] describing whether a legacy format was present -/// and, if so, whether it was compatible. `Err` is only returned for genuine IO -/// failures while persisting the migrated format. +/// and, if so, whether it was compatible. `Err` is returned for read or persist +/// failures that prevent a conclusive migration decision. async fn try_migrate_format( disks: &[Option], + rustfs_formats: &[Option], set_count: usize, set_drive_count: usize, ) -> Result { + let legacy_format_max_bytes = legacy_format_max_bytes(disks.len())?; let mut legacy_seen = false; + let mut invalid_legacy_format = false; + let mut read_error: Option = None; + let mut legacy_formats = vec![None; disks.len()]; - for disk in disks.iter().flatten() { - let data = match disk.read_all(MIGRATING_META_BUCKET, FORMAT_CONFIG_FILE).await { - Ok(d) if !d.is_empty() => d, - _ => continue, + let reads = stream::iter(disks.iter().enumerate().map(|(index, disk)| async move { + let Some(disk) = disk else { + return (index, None); + }; + let reader = match disk.read_file(MIGRATING_META_BUCKET, FORMAT_CONFIG_FILE).await { + Ok(reader) => reader, + Err(DiskError::FileNotFound | DiskError::VolumeNotFound) => return (index, None), + Err(err) => return (index, Some(Err(err))), + }; + let data = match read_legacy_format_bytes(reader, legacy_format_max_bytes).await { + Ok(data) => data, + Err(err) => return (index, Some(Err(err))), + }; + if data.is_empty() { + return (index, Some(Ok(None))); + } + + match FormatV3::try_from(data.as_slice()) { + Ok(format) => (index, Some(Ok(Some(format)))), + Err(err) => { + warn!( + event = "legacy_format_decode_failed", + component = "ecstore", + subsystem = "store_init", + disk_index = index, + error = %err, + "failed to decode legacy storage format" + ); + (index, Some(Ok(None))) + } + } + })) + .buffer_unordered(LEGACY_FORMAT_READ_CONCURRENCY); + tokio::pin!(reads); + while let Some((index, read)) = reads.next().await { + let Some(read) = read else { + continue; }; - // A non-empty MinIO format.json exists on at least one disk. legacy_seen = true; - - let fm = match FormatV3::try_from(data.as_ref()) { - Ok(fm) => fm, - Err(e) => { - warn!("failed to parse MinIO format.json, skipping this disk: {e}"); - continue; + match read { + Ok(Some(format)) => legacy_formats[index] = Some(format), + Ok(None) => invalid_legacy_format = true, + Err(err) => { + read_error.get_or_insert_with(|| err.into()); } - }; - - let Some(first_set) = fm.erasure.sets.first() else { - warn!("MinIO format.json has empty erasure.sets, skipping this disk"); - continue; - }; - if fm.erasure.sets.len() != set_count || first_set.len() != set_drive_count { - warn!( - "MinIO format topology mismatch: got {}x{}, expected {}x{}; skipping migration for this disk", - fm.erasure.sets.len(), - first_set.len(), - set_count, - set_drive_count - ); - continue; } - - if fm.erasure.version != FormatErasureVersion::V3 { - warn!( - "MinIO format erasure version is not V3 ({:?}); skipping migration for this disk", - fm.erasure.version - ); - continue; - } - - let mut fms = vec![None; disks.len()]; - for (idx, disk_opt) in disks.iter().enumerate() { - if disk_opt.is_none() { - continue; - } - let set_idx = idx / set_drive_count; - let disk_idx = idx % set_drive_count; - if set_idx >= fm.erasure.sets.len() || disk_idx >= fm.erasure.sets[set_idx].len() { - continue; - } - let mut newfm = fm.clone(); - newfm.erasure.this = fm.erasure.sets[set_idx][disk_idx]; - fms[idx] = Some(newfm); - } - - save_format_file_all(disks, &fms).await?; - return Ok(LegacyFormatOutcome::Migrated(Box::new(get_format_erasure_in_quorum(&fms)?))); } - Ok(if legacy_seen { - LegacyFormatOutcome::Incompatible - } else { - LegacyFormatOutcome::None + if let Some(err) = read_error { + return Err(err); + } + if !legacy_seen { + return Ok(LegacyFormatOutcome::None); + } + if invalid_legacy_format { + return Ok(LegacyFormatOutcome::Incompatible); + } + + let format = match get_format_erasure_in_quorum(&legacy_formats, 0) { + Ok(format) => format, + Err(_) => return Ok(LegacyFormatOutcome::Incompatible), + }; + if format.erasure.sets.len() != set_count + || check_format_erasure_value_for_topology(&format, disks.len(), set_drive_count).is_err() + || !formats_match_reference_slots(&legacy_formats, &format, 0) + || !formats_match_reference_slots(rustfs_formats, &format, 0) + { + return Ok(LegacyFormatOutcome::Incompatible); + } + + let mut formats_to_write = vec![None; disks.len()]; + for (index, disk) in disks.iter().enumerate() { + if disk.is_some() { + let mut disk_format = format.clone(); + disk_format.erasure.this = format.erasure.sets[index / set_drive_count][index % set_drive_count]; + formats_to_write[index] = Some(disk_format); + } + } + + let writes = disks + .iter() + .zip(&formats_to_write) + .zip(rustfs_formats) + .filter(|&((disk, _), existing)| disk.is_some() && existing.is_none()) + .map(|((disk, format), _)| save_format_file(disk, format)); + let mut write_error = None; + for result in join_all(writes).await { + if let Err(error) = result { + write_error.get_or_insert(error); + } + } + for disk in disks.iter().flatten() { + disk.set_disk_id(None).await?; + } + + let (persisted_formats, persisted_errors) = load_format_erasure_all(disks, false).await; + if let Some(error) = persisted_errors + .into_iter() + .flatten() + .find(|error| !matches!(error, DiskError::UnformattedDisk | DiskError::DiskNotFound)) + { + return match error { + DiskError::CorruptedFormat | DiskError::CorruptedBackend | DiskError::InconsistentDisk => { + Ok(LegacyFormatOutcome::Incompatible) + } + error => Err(error.into()), + }; + } + if !formats_match_reference_slots(&persisted_formats, &format, 0) { + return Ok(LegacyFormatOutcome::Incompatible); + } + let (persisted_format, quorum_members) = match select_format_erasure_in_quorum(&persisted_formats, 0) { + Ok(selected) => selected, + Err(error) => return Err(write_error.map_or(error, Into::into)), + }; + check_format_erasure_value_for_topology(&persisted_format, disks.len(), set_drive_count)?; + + Ok(LegacyFormatOutcome::Migrated { + format: Box::new(persisted_format), + quorum_members, }) } -pub fn get_format_erasure_in_quorum(formats: &[Option]) -> Result { - let mut countmap = HashMap::new(); +fn legacy_format_max_bytes(disk_count: usize) -> Result { + disk_count + .checked_mul(LEGACY_FORMAT_MAX_BYTES_PER_DISK) + .and_then(|size| size.checked_add(LEGACY_FORMAT_BASE_MAX_BYTES)) + .ok_or_else(|| Error::other("legacy format size limit overflow")) +} - for f in formats.iter() { - if f.is_some() { - let ds = f.as_ref().unwrap().drives(); - let v = countmap.entry(ds); - match v { - Entry::Occupied(mut entry) => *entry.get_mut() += 1, - Entry::Vacant(vacant) => { - vacant.insert(1); - } - }; +async fn read_legacy_format_bytes(reader: impl tokio::io::AsyncRead + Unpin, max_bytes: usize) -> disk::error::Result> { + let read_limit = max_bytes.checked_add(1).ok_or(DiskError::CorruptedFormat)?; + let mut data = Vec::with_capacity(read_limit.min(64 * 1024)); + reader + .take(u64::try_from(read_limit).map_err(|_| DiskError::CorruptedFormat)?) + .read_to_end(&mut data) + .await?; + if data.len() > max_bytes { + return Err(DiskError::CorruptedFormat); + } + Ok(data) +} + +pub(crate) fn formats_match_reference_slots(formats: &[Option], reference: &FormatV3, slot_offset: usize) -> bool { + let Ok(set_drive_count) = validate_format_erasure_layout(reference) else { + return false; + }; + + formats.iter().enumerate().all(|(index, format)| { + format.as_ref().is_none_or(|format| { + format.shared_identity() == reference.shared_identity() + && slot_offset + .checked_add(index) + .and_then(|slot| { + reference + .erasure + .sets + .get(slot / set_drive_count) + .and_then(|set| set.get(slot % set_drive_count)) + }) + .is_some_and(|expected| *expected == format.erasure.this) + }) + }) +} + +pub fn get_format_erasure_in_quorum(formats: &[Option], slot_offset: usize) -> Result { + select_format_erasure_in_quorum(formats, slot_offset).map(|(format, _)| format) +} + +pub(crate) fn select_format_erasure_in_quorum(formats: &[Option], slot_offset: usize) -> Result<(FormatV3, Vec)> { + let mut candidates = HashMap::new(); + let formats_present = formats.iter().flatten().count(); + let required_votes = formats.len() / 2 + 1; + + for (index, format) in formats + .iter() + .enumerate() + .filter_map(|(index, format)| Some((index, format.as_ref()?))) + { + let Some(slot) = slot_offset.checked_add(index) else { + continue; + }; + let (representative, set_drive_count, member_indices) = candidates + .entry(format.shared_identity()) + .or_insert_with(|| (format, validate_format_erasure_layout(format).ok(), Vec::new())); + let Some(set_drive_count) = *set_drive_count else { + continue; + }; + if representative + .erasure + .sets + .get(slot / set_drive_count) + .and_then(|set| set.get(slot % set_drive_count)) + .is_some_and(|expected| *expected == format.erasure.this) + { + member_indices.push(index); } } - let (max_drives, max_count) = countmap.iter().max_by_key(|&(_, c)| c).unwrap_or((&0, &0)); + let candidate_groups = candidates + .values() + .filter(|(_, _, member_indices)| !member_indices.is_empty()) + .count(); + let log_quorum_failure = |max_votes| { + warn!( + event = "format_quorum_failed", + component = "ecstore", + subsystem = "store_init", + state = "rejected", + formats_total = formats.len(), + formats_present, + candidate_groups, + max_votes, + required_votes, + "storage format quorum not reached" + ); + }; + let Some((format, _, member_indices)) = candidates + .into_values() + .max_by_key(|(_, _, member_indices)| member_indices.len()) + else { + log_quorum_failure(0); + return Err(Error::ErasureReadQuorum); + }; - if *max_drives == 0 || *max_count <= formats.len() / 2 { - warn!("get_format_erasure_in_quorum fi: {:?}", &formats); + let max_count = member_indices.len(); + if max_count < required_votes { + log_quorum_failure(max_count); return Err(Error::ErasureReadQuorum); } - let format = formats - .iter() - .find(|f| f.as_ref().is_some_and(|v| v.drives().eq(max_drives))) - .ok_or(Error::ErasureReadQuorum)?; - - let mut format = format.as_ref().unwrap().clone(); + let mut format = (*format).clone(); format.erasure.this = Uuid::nil(); + format.disk_info = None; - Ok(format) + let mut member_mask = vec![false; formats.len()]; + for index in member_indices { + member_mask[index] = true; + } + + Ok((format, member_mask)) } pub fn check_format_erasure_values( @@ -298,32 +513,34 @@ pub fn check_format_erasure_values( // disks: &Vec>, set_drive_count: usize, ) -> Result<()> { - for f in formats.iter() { - if f.is_none() { - continue; + let mut checked_identities = HashSet::new(); + for format in formats.iter().flatten() { + // `shared_identity` contains every persisted field inspected by the + // validators; only the per-slot UUID and runtime-only disk info differ. + if checked_identities.insert(format.shared_identity()) { + check_format_erasure_value_for_topology(format, formats.len(), set_drive_count)?; } + } + Ok(()) +} - let f = f.as_ref().unwrap(); - - check_format_erasure_value(f)?; - - let first_set = f.erasure.sets.first().ok_or_else(|| Error::other("erasure.sets is empty"))?; - - if formats.len() != f.erasure.sets.len() * first_set.len() { - return Err(Error::other(format!( - "formats length for erasure.sets does not match: got {}, expected {}", - formats.len(), - f.erasure.sets.len() * first_set.len() - ))); - } - - if first_set.len() != set_drive_count { - return Err(Error::other(format!( - "erasure set length for set_drive_count does not match: got {}, expected {}", - first_set.len(), - set_drive_count - ))); - } +fn check_format_erasure_value_for_topology(format: &FormatV3, format_count: usize, set_drive_count: usize) -> Result<()> { + let set_drive_count_in_format = validate_format_erasure_layout(format)?; + let format_drive_count = format + .erasure + .sets + .len() + .checked_mul(set_drive_count_in_format) + .ok_or_else(|| Error::other("erasure set drive count overflow"))?; + if format_count != format_drive_count { + return Err(Error::other(format!( + "formats length for erasure.sets does not match: got {format_count}, expected {format_drive_count}" + ))); + } + if set_drive_count_in_format != set_drive_count { + return Err(Error::other(format!( + "erasure set length for set_drive_count does not match: got {set_drive_count_in_format}, expected {set_drive_count}" + ))); } Ok(()) } @@ -333,12 +550,53 @@ fn check_format_erasure_value(format: &FormatV3) -> Result<()> { return Err(Error::other("invalid FormatMetaVersion")); } + if !matches!(format.format, FormatBackend::Erasure | FormatBackend::ErasureSingle) { + return Err(Error::other("invalid FormatBackend")); + } + + if format.id.is_nil() || format.id == Uuid::max() { + return Err(Error::other("invalid deployment ID")); + } + if format.erasure.version != FormatErasureVersion::V3 { return Err(Error::other("invalid FormatErasureVersion")); } Ok(()) } +fn validate_format_erasure_layout(format: &FormatV3) -> Result { + check_format_erasure_value(format)?; + + let set_drive_count = format + .erasure + .sets + .first() + .map(Vec::len) + .filter(|count| *count > 0) + .ok_or_else(|| Error::other("erasure.sets must contain at least one drive"))?; + if format.erasure.sets.iter().any(|set| set.len() != set_drive_count) { + return Err(Error::other("erasure.sets must be rectangular")); + } + let drive_count = format + .erasure + .sets + .len() + .checked_mul(set_drive_count) + .ok_or_else(|| Error::other("erasure set drive count overflow"))?; + let mut disk_ids = HashSet::with_capacity(drive_count); + + for disk_id in format.erasure.sets.iter().flatten() { + if disk_id.is_nil() || *disk_id == Uuid::max() { + return Err(Error::other("erasure.sets contains an invalid disk UUID")); + } + if !disk_ids.insert(*disk_id) { + return Err(Error::other("erasure.sets contains a duplicate disk UUID")); + } + } + + Ok(set_drive_count) +} + // load_format_erasure_all reads all format.json files pub async fn load_format_erasure_all(disks: &[Option], heal: bool) -> (Vec>, Vec>) { let mut futures = Vec::with_capacity(disks.len()); @@ -356,13 +614,9 @@ pub async fn load_format_erasure_all(disks: &[Option], heal: bool) -> } let results = join_all(futures).await; - for (i, result) in results.into_iter().enumerate() { + for result in results { match result { Ok(s) => { - if !heal { - let _ = disks[i].as_ref().unwrap().set_disk_id(Some(s.erasure.this)).await; - } - datas.push(Some(s)); errors.push(None); } @@ -483,6 +737,116 @@ pub fn ec_drives_no_config(set_drive_count: usize) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::layout::endpoint::Endpoint; + use crate::runtime::sources::{clear_local_disk_id_map_for_test, local_disk_path_by_id}; + use serial_test::serial; + + async fn local_disks(count: usize) -> (tempfile::TempDir, Vec>) { + let temp_dir = tempfile::tempdir().expect("temporary disk root should be created"); + let mut endpoints = Vec::with_capacity(count); + for disk_index in 0..count { + let path = temp_dir.path().join(format!("disk-{disk_index}")); + tokio::fs::create_dir_all(&path) + .await + .expect("temporary disk path should be created"); + let mut endpoint = + Endpoint::try_from(path.to_str().expect("temporary disk path should be UTF-8")).expect("endpoint should parse"); + endpoint.set_pool_index(0); + endpoint.set_set_index(0); + endpoint.set_disk_index(disk_index); + endpoints.push(endpoint); + } + + let (disks, errors) = init_disks( + &Endpoints::from(endpoints), + &DiskOption { + cleanup: false, + health_check: false, + }, + ) + .await; + assert!(errors.iter().all(Option::is_none), "local disk initialization failed: {errors:?}"); + + (temp_dir, disks) + } + + async fn two_local_disks_with_missing_third() -> (tempfile::TempDir, Vec>) { + let (temp_dir, mut disks) = local_disks(2).await; + disks.push(None); + + (temp_dir, disks) + } + + async fn write_legacy_format(disk: &Option, format: &FormatV3) { + write_legacy_bytes(disk, bytes::Bytes::from(format.to_json().expect("legacy format should serialize"))).await; + } + + async fn write_legacy_majority(disks: &[Option], format: &FormatV3) { + for (index, disk) in disks.iter().enumerate().take(disks.len() / 2 + 1) { + let mut disk_format = format.clone(); + disk_format.erasure.this = format.erasure.sets[0][index]; + write_legacy_format(disk, &disk_format).await; + } + } + + async fn write_legacy_bytes(disk: &Option, data: bytes::Bytes) { + let disk = disk.as_ref().expect("legacy disk should exist"); + if let Err(err) = disk.make_volume(MIGRATING_META_BUCKET).await { + assert_eq!(err, DiskError::VolumeExists, "legacy metadata volume should be created"); + } + disk.write_all(MIGRATING_META_BUCKET, FORMAT_CONFIG_FILE, data) + .await + .expect("legacy format should be written"); + } + + async fn write_rustfs_bytes(disk: &Option, data: bytes::Bytes) { + let disk = disk.as_ref().expect("RustFS disk should exist"); + if let Err(err) = disk.make_volume(RUSTFS_META_BUCKET).await { + assert_eq!(err, DiskError::VolumeExists, "RustFS metadata volume should be created"); + } + disk.write_all(RUSTFS_META_BUCKET, FORMAT_CONFIG_FILE, data) + .await + .expect("RustFS format should be written"); + } + + #[tokio::test] + async fn legacy_format_reader_rejects_oversized_input() { + let max_bytes = 32; + let accepted = read_legacy_format_bytes(std::io::Cursor::new(vec![0; max_bytes]), max_bytes) + .await + .expect("input at the limit should be accepted"); + assert_eq!(accepted.len(), max_bytes); + + let err = read_legacy_format_bytes(std::io::Cursor::new(vec![0; max_bytes + 1]), max_bytes) + .await + .expect_err("input above the limit must be rejected before parsing"); + assert_eq!(err, DiskError::CorruptedFormat); + } + + #[tokio::test] + async fn oversized_legacy_format_fails_closed_without_rustfs_writes() { + let (_temp_dir, mut disks) = local_disks(3).await; + let legacy = FormatV3::new(1, 3); + let oversized_len = legacy_format_max_bytes(disks.len()).expect("size limit should resolve") + 1; + for (index, disk) in disks.iter().enumerate().take(2) { + let mut disk_format = legacy.clone(); + disk_format.erasure.this = legacy.erasure.sets[0][index]; + let mut data = disk_format.to_json().expect("legacy format should serialize").into_bytes(); + data.resize(oversized_len, b' '); + write_legacy_bytes(disk, data.into()).await; + } + + let error = connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect_err("oversized legacy metadata must be rejected"); + assert!(matches!(error, Error::CorruptedFormat), "unexpected error: {error:?}"); + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "oversized legacy metadata must not be migrated"); + assert!( + errors.iter().all(|error| matches!(error, Some(DiskError::UnformattedDisk))), + "oversized legacy metadata must not create RustFS format files: {errors:?}" + ); + } #[test] fn ec_drives_no_config_uses_topology_defaults() { @@ -490,6 +854,579 @@ mod tests { assert_eq!(ec_drives_no_config(2).expect("two-drive topology should resolve"), 1); assert_eq!(ec_drives_no_config(6).expect("six-drive topology should resolve"), 3); } + + #[test] + fn format_quorum_rejects_duplicate_sentinel_and_ragged_layouts() { + let mut duplicate = FormatV3::new(1, 3); + duplicate.erasure.sets[0][1] = duplicate.erasure.sets[0][0]; + + let mut nil = FormatV3::new(1, 3); + nil.erasure.sets[0][2] = Uuid::nil(); + + let mut max = FormatV3::new(1, 3); + max.erasure.sets[0][2] = Uuid::max(); + + let mut ragged = FormatV3::new(2, 2); + ragged.erasure.sets[1].pop(); + + for (name, format, total_slots, voting_slots) in [ + ("duplicate", duplicate, 3, vec![0, 1]), + ("nil", nil, 3, vec![0, 1]), + ("max", max, 3, vec![0, 1]), + ("ragged", ragged, 4, vec![0, 1, 2]), + ] { + let set_drive_count = format.erasure.sets[0].len(); + let mut formats = vec![None; total_slots]; + for index in voting_slots { + let mut vote = format.clone(); + vote.erasure.this = format.erasure.sets[index / set_drive_count][index % set_drive_count]; + formats[index] = Some(vote); + } + + assert!( + matches!(get_format_erasure_in_quorum(&formats, 0), Err(Error::ErasureReadQuorum)), + "{name} layout must not form a format quorum" + ); + assert!( + check_format_erasure_values(&formats, set_drive_count).is_err(), + "{name} layout must fail startup format validation" + ); + } + } + + #[test] + fn format_quorum_rejects_unknown_backend_and_invalid_deployment_ids() { + let mut unknown_backend = FormatV3::new(1, 3); + unknown_backend.format = FormatBackend::Unknown; + + let mut nil_deployment = FormatV3::new(1, 3); + nil_deployment.id = Uuid::nil(); + + let mut max_deployment = FormatV3::new(1, 3); + max_deployment.id = Uuid::max(); + + for (name, format) in [ + ("unknown backend", unknown_backend), + ("nil deployment ID", nil_deployment), + ("max deployment ID", max_deployment), + ] { + let mut formats = vec![None; 3]; + for (index, slot) in formats.iter_mut().take(2).enumerate() { + let mut vote = format.clone(); + vote.erasure.this = format.erasure.sets[0][index]; + *slot = Some(vote); + } + + assert!( + matches!(get_format_erasure_in_quorum(&formats, 0), Err(Error::ErasureReadQuorum)), + "{name} must not form a format quorum" + ); + } + } + + #[test] + fn reference_slot_validation_rejects_wrong_slot_and_foreign_minorities() { + let reference = FormatV3::new(1, 3); + let mut formats = (0..3) + .map(|index| { + let mut format = reference.clone(); + format.erasure.this = reference.erasure.sets[0][index]; + Some(format) + }) + .collect::>(); + assert!(formats_match_reference_slots(&formats, &reference, 0)); + + formats[2].as_mut().expect("third format should exist").erasure.this = reference.erasure.sets[0][0]; + assert!(!formats_match_reference_slots(&formats, &reference, 0)); + + formats[2] = { + let mut foreign = reference.clone(); + foreign.id = Uuid::new_v4(); + foreign.erasure.this = foreign.erasure.sets[0][2]; + Some(foreign) + }; + assert!(!formats_match_reference_slots(&formats, &reference, 0)); + } + + #[tokio::test] + async fn existing_format_load_succeeds_with_a_strict_majority() { + let (_temp_dir, mut disks) = two_local_disks_with_missing_third().await; + let mut format = FormatV3::new(1, 3); + let mut expected = format.clone(); + expected.erasure.this = Uuid::nil(); + + format.erasure.this = format.erasure.sets[0][0]; + save_format_file(&disks[0], &Some(format.clone())) + .await + .expect("existing format should be written to the first disk"); + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::ErasureReadQuorum) + )); + assert!(matches!( + load_format_erasure(disks[1].as_ref().expect("second disk should exist"), false).await, + Err(DiskError::UnformattedDisk) + )); + + format.erasure.this = format.erasure.sets[0][1]; + save_format_file(&disks[1], &Some(format)) + .await + .expect("existing format should be written to the second disk"); + + assert_eq!( + connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("two existing formats should satisfy the production load path"), + expected + ); + } + + #[tokio::test] + async fn existing_format_load_rejects_conflicting_formats_without_a_majority() { + let (_temp_dir, mut disks) = two_local_disks_with_missing_third().await; + let mut first = FormatV3::new(1, 3); + first.erasure.this = first.erasure.sets[0][0]; + save_format_file(&disks[0], &Some(first)) + .await + .expect("first existing format should be written"); + + let mut second = FormatV3::new(1, 3); + second.erasure.this = second.erasure.sets[0][1]; + save_format_file(&disks[1], &Some(second)) + .await + .expect("conflicting existing format should be written"); + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::ErasureReadQuorum) + )); + } + + #[tokio::test] + async fn existing_format_load_excludes_disks_outside_the_selected_quorum() { + for wrong_slot_id in [false, true] { + let (_temp_dir, mut disks) = local_disks(3).await; + let mut majority = FormatV3::new(1, 3); + let mut expected = majority.clone(); + expected.erasure.this = Uuid::nil(); + + for (index, disk) in disks.iter().take(2).enumerate() { + majority.erasure.this = majority.erasure.sets[0][index]; + save_format_file(disk, &Some(majority.clone())) + .await + .expect("majority format should be written"); + } + + let mut outlier = if wrong_slot_id { + majority.clone() + } else { + FormatV3::new(1, 3) + }; + outlier.erasure.this = if wrong_slot_id { + outlier.erasure.sets[0][0] + } else { + outlier.erasure.sets[0][2] + }; + save_format_file(&disks[2], &Some(outlier)) + .await + .expect("outlier format should be written"); + + assert_eq!( + connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("two valid members should select the majority format"), + expected + ); + assert!(disks[0].is_some() && disks[1].is_some()); + assert!(disks[2].is_none(), "the outlier disk must not enter the selected erasure set"); + } + } + + #[tokio::test] + #[serial] + async fn existing_format_load_publishes_only_validated_quorum_disk_ids() { + clear_local_disk_id_map_for_test().await; + let (_temp_dir, mut disks) = local_disks(5).await; + let canonical = FormatV3::new(1, 5); + for (index, disk) in disks.iter().enumerate().take(3) { + let mut disk_format = canonical.clone(); + disk_format.erasure.this = canonical.erasure.sets[0][index]; + save_format_file(disk, &Some(disk_format)) + .await + .expect("canonical format should be written"); + } + let mut wrong_slot = canonical.clone(); + wrong_slot.erasure.this = wrong_slot.erasure.sets[0][0]; + save_format_file(&disks[3], &Some(wrong_slot)) + .await + .expect("wrong-slot format should be written"); + let mut foreign = FormatV3::new(1, 5); + foreign.erasure.this = foreign.erasure.sets[0][4]; + let foreign_id = foreign.erasure.this; + save_format_file(&disks[4], &Some(foreign)) + .await + .expect("foreign format should be written"); + let canonical_id = canonical.erasure.sets[0][0]; + let canonical_endpoint = disks[0].as_ref().expect("canonical disk should exist").endpoint().to_string(); + + connect_load_init_formats(true, &mut disks, 1, 5, None) + .await + .expect("the canonical strict majority should load"); + + assert!(disks[..3].iter().all(Option::is_some)); + assert!(disks[3..].iter().all(Option::is_none)); + assert_eq!(local_disk_path_by_id(&canonical_id).await, Some(canonical_endpoint)); + assert_eq!(local_disk_path_by_id(&foreign_id).await, None); + clear_local_disk_id_map_for_test().await; + } + + #[tokio::test] + async fn existing_format_load_excludes_a_malformed_outlier() { + let (_temp_dir, mut disks) = local_disks(3).await; + let mut majority = FormatV3::new(1, 3); + let mut expected = majority.clone(); + expected.erasure.this = Uuid::nil(); + + for (index, disk) in disks.iter().take(2).enumerate() { + majority.erasure.this = majority.erasure.sets[0][index]; + save_format_file(disk, &Some(majority.clone())) + .await + .expect("majority format should be written"); + } + let mut malformed = majority; + malformed.erasure.sets[0][1] = malformed.erasure.sets[0][0]; + malformed.erasure.this = malformed.erasure.sets[0][2]; + save_format_file(&disks[2], &Some(malformed)) + .await + .expect("malformed outlier should be written"); + + assert_eq!( + connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("one malformed outlier must not block a valid strict majority"), + expected + ); + assert!(disks[0].is_some() && disks[1].is_some()); + assert!(disks[2].is_none(), "the malformed outlier must be isolated"); + } + + #[tokio::test] + async fn fresh_format_load_does_not_initialize_with_a_missing_disk() { + let (_temp_dir, mut disks) = two_local_disks_with_missing_third().await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::FirstDiskWait) + )); + assert!(matches!( + connect_load_init_formats(false, &mut disks, 1, 3, None).await, + Err(Error::NotFirstDisk) + )); + + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none)); + assert!(matches!( + errors.as_slice(), + [ + Some(DiskError::UnformattedDisk), + Some(DiskError::UnformattedDisk), + Some(DiskError::DiskNotFound) + ] + )); + } + + #[tokio::test] + async fn compatible_legacy_format_migrates() { + let (_temp_dir, mut disks) = local_disks(3).await; + let legacy = FormatV3::new(1, 3); + write_legacy_majority(&disks, &legacy).await; + + let mut expected = legacy; + expected.erasure.this = Uuid::nil(); + assert_eq!( + connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("compatible legacy format should migrate"), + expected + ); + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!( + errors.iter().all(Option::is_none), + "every available disk should receive the migrated format" + ); + for (index, format) in formats.iter().enumerate() { + assert_eq!( + format.as_ref().map(|format| format.erasure.this), + Some(expected.erasure.sets[0][index]), + "the migrated format must preserve each disk's physical slot" + ); + } + } + + #[tokio::test] + async fn compatible_legacy_format_migrates_when_the_file_is_missing() { + let (_temp_dir, mut disks) = local_disks(3).await; + let legacy = FormatV3::new(1, 3); + write_legacy_majority(&disks, &legacy).await; + disks[2] + .as_ref() + .expect("third legacy disk should exist") + .make_volume(MIGRATING_META_BUCKET) + .await + .expect("legacy metadata volume should be created without a format file"); + + connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("a missing legacy format file should be initialized from the majority"); + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!( + errors.iter().all(Option::is_none), + "every available disk should receive the migrated format" + ); + assert_eq!( + formats[2].as_ref().map(|format| format.erasure.this), + Some(legacy.erasure.sets[0][2]), + "the missing legacy format file must be replaced with the third slot" + ); + } + + #[tokio::test] + async fn legacy_format_migration_resumes_partial_rustfs_writes() { + for drive_count in [3, 4] { + let (_temp_dir, mut disks) = local_disks(drive_count).await; + let legacy = FormatV3::new(1, drive_count); + write_legacy_majority(&disks, &legacy).await; + for (index, disk) in disks.iter().enumerate().take(drive_count / 2) { + let mut disk_format = legacy.clone(); + disk_format.erasure.this = legacy.erasure.sets[0][index]; + save_format_file(disk, &Some(disk_format)) + .await + .expect("partial RustFS format should be written"); + } + + let mut expected = legacy; + expected.erasure.this = Uuid::nil(); + assert_eq!( + connect_load_init_formats(true, &mut disks, 1, drive_count, None) + .await + .expect("a partial migration should resume from the legacy quorum"), + expected + ); + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(errors.iter().all(Option::is_none), "the resumed migration should format every disk"); + for (index, format) in formats.iter().enumerate() { + assert_eq!( + format.as_ref().map(|format| format.erasure.this), + Some(expected.erasure.sets[0][index]), + "the resumed migration must preserve each physical slot" + ); + } + } + } + + #[tokio::test] + async fn legacy_format_migration_resumes_to_quorum_with_an_unavailable_disk() { + let (_temp_dir, mut disks) = two_local_disks_with_missing_third().await; + let legacy = FormatV3::new(1, 3); + write_legacy_majority(&disks, &legacy).await; + let mut partial = legacy.clone(); + partial.erasure.this = partial.erasure.sets[0][0]; + save_format_file(&disks[0], &Some(partial)) + .await + .expect("partial RustFS format should be written"); + + let mut expected = legacy; + expected.erasure.this = Uuid::nil(); + assert_eq!( + connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("the available blank disk should complete the migration quorum"), + expected + ); + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(formats[0].is_some() && formats[1].is_some()); + assert!(formats[2].is_none()); + assert!(matches!(errors[2], Some(DiskError::DiskNotFound))); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_a_foreign_partial_rustfs_format() { + let (_temp_dir, mut disks) = local_disks(3).await; + let legacy = FormatV3::new(1, 3); + write_legacy_majority(&disks, &legacy).await; + let mut foreign = FormatV3::new(1, 3); + foreign.erasure.this = foreign.erasure.sets[0][0]; + let foreign_id = foreign.id; + save_format_file(&disks[0], &Some(foreign)) + .await + .expect("foreign partial RustFS format should be written"); + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert_eq!(formats[0].as_ref().map(|format| format.id), Some(foreign_id)); + assert!(formats[1..].iter().all(Option::is_none), "foreign partial state must not be extended"); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_a_wrong_slot_partial_rustfs_format() { + let (_temp_dir, mut disks) = local_disks(3).await; + let legacy = FormatV3::new(1, 3); + write_legacy_majority(&disks, &legacy).await; + let mut wrong_slot = legacy; + wrong_slot.erasure.this = wrong_slot.erasure.sets[0][1]; + save_format_file(&disks[0], &Some(wrong_slot)) + .await + .expect("wrong-slot partial RustFS format should be written"); + + let error = connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect_err("a wrong-slot partial RustFS format must be rejected"); + assert!(matches!(error, Error::CorruptedFormat), "unexpected error: {error:?}"); + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(matches!(errors[0], Some(DiskError::InconsistentDisk))); + assert!(formats[1..].iter().all(Option::is_none), "wrong-slot partial state must not be extended"); + } + + #[tokio::test] + async fn legacy_format_migration_does_not_extend_a_malformed_rustfs_format() { + let (_temp_dir, mut disks) = local_disks(3).await; + let legacy = FormatV3::new(1, 3); + write_legacy_majority(&disks, &legacy).await; + write_rustfs_bytes(&disks[0], bytes::Bytes::from_static(b"{not-json")).await; + + assert!(connect_load_init_formats(true, &mut disks, 1, 3, None).await.is_err()); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "malformed partial state must not be extended"); + } + + #[tokio::test] + async fn incompatible_legacy_format_fails_closed() { + let (_temp_dir, mut disks) = local_disks(3).await; + let mut legacy = FormatV3::new(1, 3); + legacy.id = Uuid::nil(); + write_legacy_majority(&disks, &legacy).await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "incompatible legacy metadata must not be replaced"); + assert!( + errors.iter().all(|error| matches!(error, Some(DiskError::UnformattedDisk))), + "fresh RustFS formats must not be written after a rejected migration: {errors:?}" + ); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_a_foreign_minority() { + let (_temp_dir, mut disks) = local_disks(3).await; + let majority = FormatV3::new(1, 3); + write_legacy_majority(&disks, &majority).await; + let minority = FormatV3::new(1, 3); + let mut minority_disk = minority; + minority_disk.erasure.this = minority_disk.erasure.sets[0][2]; + write_legacy_format(&disks[2], &minority_disk).await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "a foreign legacy minority must not be overwritten"); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_a_wrong_slot_minority() { + let (_temp_dir, mut disks) = local_disks(3).await; + let majority = FormatV3::new(1, 3); + write_legacy_majority(&disks, &majority).await; + let mut wrong_slot = majority; + wrong_slot.erasure.this = wrong_slot.erasure.sets[0][1]; + write_legacy_format(&disks[2], &wrong_slot).await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!( + formats.iter().all(Option::is_none), + "a wrong-slot legacy minority must not be overwritten" + ); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_a_malformed_minority() { + let (_temp_dir, mut disks) = local_disks(3).await; + let majority = FormatV3::new(1, 3); + write_legacy_majority(&disks, &majority).await; + write_legacy_bytes(&disks[2], bytes::Bytes::from_static(b"{not-json")).await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "a malformed legacy minority must not be overwritten"); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_an_empty_minority() { + let (_temp_dir, mut disks) = local_disks(3).await; + let majority = FormatV3::new(1, 3); + write_legacy_majority(&disks, &majority).await; + write_legacy_bytes(&disks[2], bytes::Bytes::new()).await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "an empty legacy minority must not be overwritten"); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_a_parseable_invalid_minority() { + let (_temp_dir, mut disks) = local_disks(3).await; + let majority = FormatV3::new(1, 3); + write_legacy_majority(&disks, &majority).await; + let mut invalid = majority; + invalid.id = Uuid::nil(); + invalid.erasure.this = invalid.erasure.sets[0][2]; + write_legacy_format(&disks[2], &invalid).await; + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!( + formats.iter().all(Option::is_none), + "a parseable invalid legacy minority must not be overwritten" + ); + } + + #[tokio::test] + async fn legacy_format_migration_rejects_conflicting_candidates_without_a_quorum() { + let (_temp_dir, mut disks) = local_disks(3).await; + for (index, disk) in disks.iter().enumerate().take(2) { + let mut disk_format = FormatV3::new(1, 3); + disk_format.erasure.this = disk_format.erasure.sets[0][index]; + write_legacy_format(disk, &disk_format).await; + } + + assert!(matches!( + connect_load_init_formats(true, &mut disks, 1, 3, None).await, + Err(Error::CorruptedFormat) + )); + let (formats, _) = load_format_erasure_all(&disks, false).await; + assert!(formats.iter().all(Option::is_none), "a legacy split vote must not write RustFS formats"); + } } // #[derive(Debug, PartialEq, thiserror::Error)] diff --git a/crates/ecstore/src/store/mod.rs b/crates/ecstore/src/store/mod.rs index 6ddb6d5bb..6c7f2523a 100644 --- a/crates/ecstore/src/store/mod.rs +++ b/crates/ecstore/src/store/mod.rs @@ -989,7 +989,7 @@ mod tests { init_local_disks(endpoint_pools.clone()).await.expect("init local disks"); - let (disks, errs) = init_disks( + let (mut disks, errs) = init_disks( &endpoint_pools.as_ref().first().expect("pool endpoints").endpoints, &DiskOption { cleanup: true, @@ -999,7 +999,7 @@ mod tests { .await; assert!(errs.iter().all(|err| err.is_none()), "disk init should succeed: {errs:?}"); - connect_load_init_formats(true, &disks, 1, 4, None) + connect_load_init_formats(true, &mut disks, 1, 4, None) .await .expect("initialize format metadata"); diff --git a/crates/utils/src/net.rs b/crates/utils/src/net.rs index ab05fea83..d9bf106f5 100644 --- a/crates/utils/src/net.rs +++ b/crates/utils/src/net.rs @@ -92,7 +92,6 @@ fn resolve_domain(domain: &str) -> std::io::Result> { (domain, 0) .to_socket_addrs() .map(|v| v.map(|v| v.ip()).collect::>()) - .map_err(Error::other) } #[cfg(test)] @@ -284,7 +283,7 @@ pub async fn get_host_ip(host: Host<&str>) -> std::io::Result> { } Err(err) => { error!("Failed to resolve domain {domain} using system resolver, err: {err}"); - Err(Error::other(err)) + Err(err) } } } @@ -608,6 +607,27 @@ mod test { assert!(get_host_ip(invalid_host).await.is_err()); } + #[tokio::test] + async fn test_get_host_ip_preserves_resolver_error_provenance() { + let _resolver_guard = set_mock_dns_resolver(|_| Err(IoError::from_raw_os_error(-3))); + + let err = get_host_ip(Host::Domain("temporarily-unavailable.example")) + .await + .unwrap_err(); + + assert_eq!(err.raw_os_error(), Some(-3)); + } + + #[test] + fn test_resolve_domain_preserves_system_resolver_error_provenance() { + let _resolver_lock = DNS_RESOLVER_TEST_LOCK.lock().unwrap(); + reset_dns_resolver_inner(); + + let err = resolve_domain("rustfs-resolver-provenance.invalid").unwrap_err(); + + assert_ne!(err.kind(), std::io::ErrorKind::Other, "system resolver error was wrapped: {err}"); + } + #[test] fn test_dns_cache_entry_expiry() { let ips: HashSet = [IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1))].into_iter().collect(); diff --git a/crates/utils/src/string.rs b/crates/utils/src/string.rs index f8f798de9..c22dbf62e 100644 --- a/crates/utils/src/string.rs +++ b/crates/utils/src/string.rs @@ -360,7 +360,7 @@ pub fn find_ellipses_patterns(arg: &str) -> Result { Some(caps) => caps, None => { return Err(Error::other(format!( - "Invalid ellipsis format in ({arg}), Ellipsis range must be provided in format {{N...M}} where N and M are decimal or hexadecimal positive integers, M must be greater than N, with a maximum expanded range size of {MAX_ELLIPSES_RANGE_SIZE}" + "Invalid ellipsis format. Ellipsis range must be provided in format {{N...M}} where N and M are decimal or hexadecimal positive integers, M must be greater than N, with a maximum expanded range size of {MAX_ELLIPSES_RANGE_SIZE}" ))); } }; @@ -399,7 +399,7 @@ pub fn find_ellipses_patterns(arg: &str) -> Result { || p.suffix.contains(CLOSE_BRACES) { return Err(Error::other(format!( - "Invalid ellipsis format in ({arg}), Ellipsis range must be provided in format {{N...M}} where N and M are decimal or hexadecimal positive integers, M must be greater than N, with a maximum expanded range size of {MAX_ELLIPSES_RANGE_SIZE}" + "Invalid ellipsis format. Ellipsis range must be provided in format {{N...M}} where N and M are decimal or hexadecimal positive integers, M must be greater than N, with a maximum expanded range size of {MAX_ELLIPSES_RANGE_SIZE}" ))); } } @@ -938,6 +938,15 @@ mod tests { assert!(err.to_string().contains("decimal or hexadecimal"), "unexpected error message: {err}"); } + #[test] + fn test_find_ellipses_patterns_leftover_brace_error_does_not_echo_input() { + let err = find_ellipses_patterns("http://:brace-secret@server/{1...2}}").unwrap_err(); + assert!( + !err.to_string().contains("brace-secret"), + "ellipsis error leaked endpoint credentials: {err}" + ); + } + #[test] fn test_parse_ellipses_range_rejects_oversized_ranges() { let err = parse_ellipses_range("{0...10000}").unwrap_err(); diff --git a/docs/architecture/compat-cleanup-register.md b/docs/architecture/compat-cleanup-register.md index 90718159b..1bdc236a3 100644 --- a/docs/architecture/compat-cleanup-register.md +++ b/docs/architecture/compat-cleanup-register.md @@ -12,6 +12,10 @@ for later deletion. ## Open Items +- `rustfs-5416` Helm distributed startup wait setting: charts that predate explicit local endpoint identity expose startupWaitTimeoutSeconds for their peer DNS/TCP init gate. The new chart keeps the value accepted but ignores it after moving startup convergence into RustFS. Remove the value and its documentation after the minimum supported direct-upgrade chart includes localEndpointHost.autoInject and no longer renders the peer gate. +- `rustfs-5416-wait-mode` startup wait-mode validation: releases before explicit local endpoint identity treat an unknown RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE value as auto. New servers retain that fallback only when no explicit local endpoint host is configured; an anchor requires a recognized mode so a typo cannot bypass DNS locality. Remove the fallback and reject every unknown value after every supported direct-upgrade chart validates this setting before rollout. +- `rustfs-5416-kubernetes-alias-dns` Kubernetes endpoint identity fallback: deployments created before explicit local endpoint identity may use resolvable aliases that do not match the Pod hostname. An implicit auto-mode zero match retains legacy DNS locality with a bounded deadline, while ambiguous matches and invalid explicit anchors still fail closed. Remove the fallback after every supported direct-upgrade chart and deployment manifest provides a canonical RUSTFS_LOCAL_ENDPOINT_HOST for domain-based distributed topologies. +- `rustfs-5416-zero-retry-delay` startup retry-delay validation: releases before bounded topology convergence accept RUSTFS_STARTUP_TOPOLOGY_RETRY_MAX_DELAY values of 0 or 0ms. New servers replace those values with the safe nonzero default so a direct upgrade neither fails startup nor enters a busy loop. Reject zero after the minimum supported direct-upgrade release validates or rewrites this setting before rollout. - `scanner-usage-v2` persisted scanner usage migration: pre-v2 scanners write `.usage.json`, so upgraded clusters read that primary/backup pair only while `.usage.v2.json` is absent and continue removing deleted buckets from legacy copies that still exist. The additive usage_snapshot_complete field in `.usage.v2.json` must remain optional while mixed-version clusters are supported; a missing field means the snapshot is not authoritative. Remove the legacy object fallback and cleanup only after every supported direct-upgrade source writes `.usage.v2.json`. - `ns-scanner-rpc-v3` namespace scanner capability and activity handshake: old peers and legacy internode transports lack the authenticated startup-epoch handshake. The oldest peers send an empty activity request and receive a field-empty protocol-0 response. Protocol v4 binds the challenge and response topology but cannot authenticate distributed dirty-usage state. Current protocol v5 binds the request version, acknowledgement target and generation, and the response dirty-usage state. Servers retain protocol-0 and protocol-v4 codecs for rolling upgrades, while the distributed scanner publishes usage only after every peer returns authenticated protocol v5 state. Scanner selection treats HTTP 404/405/426 and the legacy MethodNotAllowed default as an explicit lack of remote scanner v3 support and assigns those disks to coordinator-driven workers; transient capability failures remain incomplete and do not activate the fallback. Remove the coordinator fallback after the minimum supported RustFS peer version implements namespace scanner protocol v3, remove protocol-0 activity requests and responses after every supported peer implements authenticated scanner activity protocol v4, and remove the protocol-v4 activity codec after every supported peer implements protocol v5; future protocol revisions must keep the same dual-version server/codec window before changing the advertised version. - `#4648` walk-dir stream completion capability: old clients can append fallback output to an already-used metacache writer after a terminal body error, so servers emit terminal walk errors only to clients that sign the `walk_dir_stream_completion=error-v1` query capability and its request-body digest. Remove the legacy clean-EOF path after the minimum supported RustFS peer version always advertises this capability. diff --git a/helm/README.md b/helm/README.md index 07df2792d..148d38cda 100644 --- a/helm/README.md +++ b/helm/README.md @@ -47,6 +47,60 @@ without manual intervention: If you previously set `replicaCount=16` and now want a different topology, set both `replicaCount` and `drivesPerNode` explicitly. +For distributed deployments that use the chart-generated +`RUSTFS_VOLUMES`, `localEndpointHost.autoInject` defaults to automatic +selection. Without `secret.existingSecret`, the chart injects a private +Downward API variable, `RUSTFS_CHART_POD_NAME`, and uses it to build the pod's +fully qualified hostname in +`RUSTFS_LOCAL_ENDPOINT_HOST`. RustFS can then identify local drives without +waiting for every peer's DNS record or TCP listener. A user-defined `POD_NAME` +is preserved and does not interfere with the private chart variable. Setting +`RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE` explicitly to `bounded`, `fail-fast`, +`failfast`, or `strict` also keeps legacy DNS-based locality discovery. A +dynamically sourced wait mode keeps the legacy path because its value cannot be +validated while rendering. Otherwise, Kubernetes auto-detection selects +orchestrated startup when RustFS consumes the generated anchor. + +An existing Secret is opaque to the chart and may historically contain more +than credentials, so the chart does not inject an anchor whenever +`secret.existingSecret` is set. In Kubernetes auto/orchestrated mode, RustFS +then derives a DNS-free identity from the kernel hostname when exactly one +domain endpoint at the server port has the same full hostname or first label. +All-IP topologies retain direct IP locality detection. A domain topology with +zero matches retains legacy DNS locality discovery; implicit auto mode bounds +that compatibility path by `RUSTFS_STARTUP_TOPOLOGY_WAIT_TIMEOUT` (180 seconds +by default). Multiple matching candidates remain an error. Set +`RUSTFS_LOCAL_ENDPOINT_HOST` explicitly to avoid DNS discovery. For a +credentials-only Secret, set +`localEndpointHost.autoInject=true` to add the chart anchor without changing +the historical ConfigMap-then-Secret `envFrom` precedence. If injection is +explicitly enabled with an incompatible hidden `RUSTFS_VOLUMES`, +`RUSTFS_ADDRESS`, or `RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE`, RustFS also fails +during endpoint construction; it does not silently fall back to a different +topology. + +When `config.rustfs.volumes` is set explicitly, the chart does not infer a +local endpoint identity. RustFS applies the same kernel-hostname inference to +custom domain topologies in Kubernetes auto/orchestrated mode; aliases that do +not match the Pod hostname retain legacy DNS locality, with the bounded auto +fallback described above. They may provide `RUSTFS_LOCAL_ENDPOINT_HOST` +through `extraEnv` for DNS-free startup. An explicit `RUSTFS_VOLUMES`, explicit +`RUSTFS_LOCAL_ENDPOINT_HOST`, bounded/dynamic or unrecognized startup mode, or +`localEndpointHost.autoInject=false`, also disables chart injection. A +`RUSTFS_ADDRESS` override alone does not disable it; the effective address and +generated topology must agree on the endpoint port. Custom anchor-based +configurations must resolve to `orchestrated` startup mode and must not receive +a conflicting mode from an `envFrom` source. +`startupWaitTimeoutSeconds` is retained for values-file compatibility but is +deprecated and ignored. +Historical `RUSTFS_STARTUP_TOPOLOGY_RETRY_MAX_DELAY` values of `0` or `0ms` +are replaced with the safe default retry cap instead of causing a busy loop or +blocking a direct upgrade. + +Upgrade the chart and RustFS image together. An older image that does not +recognize `RUSTFS_LOCAL_ENDPOINT_HOST` retains its previous DNS-based startup +behavior. + --- # Parameters Overview @@ -57,6 +111,7 @@ set both `replicaCount` and `drivesPerNode` explicitly. | affinity.podAntiAffinity.enabled | bool | `true` | | | affinity.podAntiAffinity.topologyKey | string | `"kubernetes.io/hostname"` | | | clusterDomain | string | `"cluster.local"` | Kubernetes cluster DNS domain used to build in-cluster FQDNs for `RUSTFS_VOLUMES` (distributed mode) and mTLS server certificate SANs. Override for clusters not using the default `cluster.local`. Provide the DNS root only, without a `svc.` prefix or leading/trailing dots. | +| localEndpointHost.autoInject | bool or null | `null` | Automatically inject `RUSTFS_LOCAL_ENDPOINT_HOST` for chart-generated distributed topologies unless `secret.existingSecret` is set. Use `true` for a credentials-only existing Secret or `false` to preserve legacy DNS locality explicitly. | | commonLabels | object | `{}` | Labels to add to all deployed objects. | | config.rustfs.address | string | `":9000"` | | | config.rustfs.console_address | string | `":9001"` | | @@ -66,7 +121,7 @@ set both `replicaCount` and `drivesPerNode` explicitly. | config.rustfs.obs_environment | string | `"development"` | | | config.rustfs.obs_log_directory | string | `"/logs"` | Log directory inside the RustFS container. Set to `""` to disable log PVCs and mounts. | | config.rustfs.region | string | `"us-east-1"` | | -| config.rustfs.volumes | string | `""` | | +| config.rustfs.volumes | string | `""` | Explicit distributed volume topology. When empty, the chart generates the topology and normally injects `RUSTFS_LOCAL_ENDPOINT_HOST`; custom topologies must configure local endpoint identity explicitly when needed. | | config.rustfs.log_rotation.size | int | `"100"` | Default log rotation size mb for rustfs. | | config.rustfs.log_rotation.time | string | `"hour"` | Default log rotation time for rustfs. | | config.rustfs.log_rotation.keep_files | int | `"30"` | Default log keep files for rustfs. | @@ -107,7 +162,7 @@ set both `replicaCount` and `drivesPerNode` explicitly. | config.rustfs.kms.vault.vault_token | string | `""`| The vault token. Rendered into a dedicated Secret (`-kms-secret`), never into the ConfigMap. | | config.rustfs.kms.vault.vault_mount_path | string | `"transit"`| The vault mount path, only works if `vault_backend` equals `vault-transit` . | | config.rustfs.kms.vault.default_key | string | `"transit"`| The master key id for RustFS. | -| extraEnv | map | `[]` | Extra environment variables for RustFS container. | +| extraEnv | list | `[]` | Extra environment variables for the RustFS container. An explicit `RUSTFS_LOCAL_ENDPOINT_HOST` or `RUSTFS_VOLUMES`, or a bounded, dynamic, or unrecognized startup mode, disables generated anchor injection. `POD_NAME` and `RUSTFS_ADDRESS` remain independent overrides. | | extraVolumes | list | `[]` | Extra volumes to add to the pod spec. Supported in both standalone (Deployment) and distributed (StatefulSet) modes. | | extraVolumeMounts | list | `[]` | Extra volume mounts to add to the RustFS container. Supported in both standalone (Deployment) and distributed (StatefulSet) modes. | | containerSecurityContext.capabilities.drop[0] | string | `"ALL"` | | @@ -190,7 +245,7 @@ uer. `ClusterIssuer` or `Issuer`. | | resources.limits.memory | string | `"512Mi"` | | | resources.requests.cpu | string | `"100m"` | | | resources.requests.memory | string | `"128Mi"` | | -| secret.existingSecret | string | `""` | Use existing secret with a credentials. | +| secret.existingSecret | string | `""` | Use an existing Secret. Automatic endpoint-anchor injection is disabled because the Secret is opaque; set `localEndpointHost.autoInject=true` only after confirming it contains credentials rather than runtime topology, address, or startup-mode overrides. | | secret.rustfs.access_key | string | `"rustfsadmin"` | RustFS Access Key ID | | secret.rustfs.secret_key | string | `"rustfsadmin"` | RustFS Secret Key ID | | service.type | string | `"ClusterIP"` | | @@ -202,6 +257,7 @@ uer. `ClusterIssuer` or `Issuer`. | | serviceAccount.automount | bool | `true` | | | serviceAccount.create | bool | `true` | | | serviceAccount.name | string | `""` | | +| startupWaitTimeoutSeconds | int | `300` | Deprecated and ignored; retained for values-file compatibility. | | storageclass.dataStorageSize | string | `"256Mi"` | The storage size for data PVC. | | storageclass.logStorageSize | string | `"256Mi"` | The storage size for logs PVC. | | storageclass.name | string | `"local-path"` | The name for StorageClass. | @@ -291,20 +347,17 @@ Notes: * **Pools are append-only.** The list index determines the StatefulSet name — never remove or reorder entries. Retire a pool with `rc admin decommission` before removing it from the list. -* During the expansion rollout, pods restart until every pod of every pool is - resolvable — the server refuses to start with unresolvable peers, so expect - a few crash/restart cycles before the cluster converges. This is harmless. +* With chart-generated volumes, each pod receives an explicit local endpoint + identity. An unavailable peer no longer blocks a pod from reaching RustFS's + own startup and quorum checks. * After the cluster converges, run `rc admin rebalance start ` to spread existing objects across the new pool. * Pod anti-affinity in pool mode is scoped per pool and **preferred** (soft), not required: two pools can share nodes, and each pool's own pods - spread across distinct nodes when capacity allows. Soft affinity is - load-bearing for in-place expansion — with required rules, the - not-yet-rolled pods of the existing pool block the new pool's pods from - their nodes while the rolled pods crash on the unresolvable (Pending) - peers, deadlocking the rollout on any cluster with fewer nodes than the - total pod count. Single-pool deployments (`pools.enabled=false`) keep the - chart's existing required anti-affinity unchanged. + spread across distinct nodes when capacity allows. Preferred affinity keeps + additional pools schedulable when the cluster has fewer nodes than total + pods. Single-pool deployments (`pools.enabled=false`) keep the chart's + existing required anti-affinity unchanged. * The PodDisruptionBudget spans all pools: with the default `pdb.maxUnavailable: 1`, at most one pod of the whole cluster may be evicted at a time. This is deliberately conservative — quorum safety @@ -315,7 +368,8 @@ Notes: ## Requirement * Helm V3 -* RustFS >= 1.0.0-alpha.69 +* The RustFS image from the same release as the chart. If `image.rustfs.tag` + is overridden, that image must support `RUSTFS_LOCAL_ENDPOINT_HOST`. Due to the traefik and ingress has different session sticky/affinity annotations, and rustfs support both those two controller, you should specify parameter `ingress.className` to select the right one which suits for you. diff --git a/helm/rustfs/templates/_helpers.tpl b/helm/rustfs/templates/_helpers.tpl index f9156b834..126e07e4f 100644 --- a/helm/rustfs/templates/_helpers.tpl +++ b/helm/rustfs/templates/_helpers.tpl @@ -322,21 +322,23 @@ Render RUSTFS_SERVER_DOMAINS {{- join "," $domains -}} {{- end -}} -{{/* Render probe command for liveness and readiness +{{/* Render an mTLS probe command */}} {{- define "rustfs.probeCommand" -}} -{{- $endpoint_port := .Values.service.endpoint.port | default 9000 -}} -{{- $console_port := .Values.service.console.port | default 9001 -}} +{{- $root := .root -}} +{{- $endpointPath := .endpointPath -}} +{{- $endpoint_port := $root.Values.service.endpoint.port | default 9000 -}} +{{- $console_port := $root.Values.service.console.port | default 9001 -}} {{- $args := "-skf" -}} -{{- if and .Values.mtls.enabled -}} - {{- $args = printf "%s --cert %s --key %s" $args .Values.mtls.clientCertPath .Values.mtls.clientKeyPath -}} +{{- if and $root.Values.mtls.enabled -}} + {{- $args = printf "%s --cert %s --key %s" $args $root.Values.mtls.clientCertPath $root.Values.mtls.clientKeyPath -}} {{- end -}} - /bin/sh - -c - | - curl {{ $args }} https://127.0.0.1:{{ $endpoint_port }}/health/ready && \ + curl {{ $args }} https://127.0.0.1:{{ $endpoint_port }}{{ $endpointPath }} && \ curl {{ $args }} https://127.0.0.1:{{ $console_port }}/rustfs/console/health {{- end -}} @@ -350,7 +352,7 @@ livenessProbe: {{- if .Values.mtls.enabled }} exec: command: -{{ include "rustfs.probeCommand" . | nindent 6 }} +{{ include "rustfs.probeCommand" (dict "root" . "endpointPath" "/health") | nindent 6 }} {{- else }} httpGet: path: /health @@ -369,7 +371,7 @@ readinessProbe: {{- if .Values.mtls.enabled }} exec: command: -{{ include "rustfs.probeCommand" . | nindent 6 }} +{{ include "rustfs.probeCommand" (dict "root" . "endpointPath" "/health/ready") | nindent 6 }} {{- else }} httpGet: path: /health/ready diff --git a/helm/rustfs/templates/statefulset.yaml b/helm/rustfs/templates/statefulset.yaml index 3954ad386..f2c90877d 100644 --- a/helm/rustfs/templates/statefulset.yaml +++ b/helm/rustfs/templates/statefulset.yaml @@ -1,12 +1,37 @@ {{- $logDir := .Values.config.rustfs.obs_log_directory }} {{- $logDirEnabled := ne $logDir "" }} {{- $poolsEnabled := and .Values.pools .Values.pools.enabled }} +{{- $generatedVolumes := empty .Values.config.rustfs.volumes }} +{{- $localEndpointHostAutoInject := empty .Values.secret.existingSecret }} +{{- with .Values.localEndpointHost }} +{{- if and (hasKey . "autoInject") (ne .autoInject nil) }} +{{- if not (kindIs "bool" .autoInject) }} +{{- fail "localEndpointHost.autoInject must be a boolean or null" }} +{{- end }} +{{- $localEndpointHostAutoInject = .autoInject }} +{{- end }} +{{- end }} +{{- $injectLocalEndpointHost := and $generatedVolumes $localEndpointHostAutoInject }} +{{- range $env := .Values.extraEnv -}} +{{- $name := default "" $env.name -}} +{{- if eq $name "RUSTFS_VOLUMES" -}} +{{- $injectLocalEndpointHost = false -}} +{{- else if eq $name "RUSTFS_LOCAL_ENDPOINT_HOST" -}} +{{- $injectLocalEndpointHost = false -}} +{{- else if eq $name "RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE" -}} +{{- if hasKey $env "value" -}} +{{- $mode := lower (trim (toString $env.value)) -}} +{{- if and (ne $mode "") (ne $mode "auto") (ne $mode "orchestrated") -}} +{{- $injectLocalEndpointHost = false -}} +{{- end -}} +{{- else -}} +{{- $injectLocalEndpointHost = false -}} +{{- end -}} +{{- end -}} +{{- end -}} {{- if and .Values.mode.distributed.enabled (le (int .Values.replicaCount) 1) -}} {{- fail "Distributed mode requires replicaCount >= 2" -}} {{- end -}} -{{- if and .Values.mode.distributed.enabled (le (int .Values.startupWaitTimeoutSeconds) 0) -}} -{{- fail "startupWaitTimeoutSeconds must be greater than 0 in distributed mode" -}} -{{- end -}} {{- if .Values.mode.distributed.enabled }} {{- $pools := include "rustfs.pools" . | fromJsonArray }} @@ -74,13 +99,10 @@ spec: {{- if $.Values.affinity.podAntiAffinity.enabled }} podAntiAffinity: {{- if $poolsEnabled }} - {{- /* Pool-scoped and PREFERRED, not required: during in-place - expansion the not-yet-rolled pods' required rules block new - pods from their nodes, while the new pods' DNS must resolve - before the roll can proceed - required rules deadlock - expansion whenever the cluster has fewer nodes than total - pods. Soft anti-affinity converges (the scheduler still - spreads when capacity allows). */}} + {{- /* Pool-scoped and PREFERRED, not required: additional pools + must remain schedulable when the cluster has fewer nodes + than total pods. The scheduler still spreads them when + capacity allows. */}} preferredDuringSchedulingIgnoredDuringExecution: - weight: 100 podAffinityTerm: @@ -127,10 +149,6 @@ spec: env: - name: DRIVES_PER_NODE value: {{ $drivesPerNode | quote }} - - name: ENDPOINT_PORT - value: {{ $.Values.service.endpoint.port | quote }} - - name: STARTUP_WAIT_TIMEOUT_SECONDS - value: {{ $.Values.startupWaitTimeoutSeconds | quote }} command: - sh - -c @@ -146,44 +164,6 @@ spec: mkdir -p /mnt/rustfs/logs chmod 755 /mnt/rustfs/logs {{- end }} - deadline=$(($(date +%s) + STARTUP_WAIT_TIMEOUT_SECONDS)) - wait_for_peer() { - description=$1 - shift - until "$@" >/dev/null 2>&1; do - if [ "$(date +%s)" -ge "$deadline" ]; then - echo "Timed out after ${STARTUP_WAIT_TIMEOUT_SECONDS}s waiting for ${description}" >&2 - exit 1 - fi - sleep 2 - done - } - - echo "Waiting for distributed peer DNS records" - {{- range $peerPool := $pools }} - {{- range $i := until (int $peerPool.replicaCount) }} - wait_for_peer "DNS for {{ $peerPool.fullname }}-{{ $i }}" \ - nslookup "{{ $peerPool.fullname }}-{{ $i }}.{{ include "rustfs.fullname" $ }}-headless.{{ $.Release.Namespace }}.svc.{{ include "rustfs.clusterDomain" $ }}" - {{- end }} - {{- end }} - - ordinal=${HOSTNAME##*-} - echo "Waiting for earlier distributed peers to listen on port ${ENDPOINT_PORT}" - {{- range $peerPool := $pools }} - {{- if lt (int $peerPool.index) (int $pool.index) }} - {{- range $i := until (int $peerPool.replicaCount) }} - wait_for_peer "{{ $peerPool.fullname }}-{{ $i }}:${ENDPOINT_PORT}" \ - nc -z -w 2 "{{ $peerPool.fullname }}-{{ $i }}.{{ include "rustfs.fullname" $ }}-headless.{{ $.Release.Namespace }}.svc.{{ include "rustfs.clusterDomain" $ }}" "$ENDPOINT_PORT" - {{- end }} - {{- else if eq (int $peerPool.index) (int $pool.index) }} - i=0 - while [ "$i" -lt "$ordinal" ]; do - wait_for_peer "{{ $peerPool.fullname }}-${i}:${ENDPOINT_PORT}" \ - nc -z -w 2 "{{ $peerPool.fullname }}-${i}.{{ include "rustfs.fullname" $ }}-headless.{{ $.Release.Namespace }}.svc.{{ include "rustfs.clusterDomain" $ }}" "$ENDPOINT_PORT" - i=$((i + 1)) - done - {{- end }} - {{- end }} volumeMounts: {{- if gt $drivesPerNode 1 }} {{- range $i := until $drivesPerNode }} @@ -210,9 +190,19 @@ spec: containerPort: {{ $.Values.service.endpoint.port }} - name: console containerPort: {{ $.Values.service.console.port }} - {{- with $.Values.extraEnv }} + {{- if or $injectLocalEndpointHost (not (empty $.Values.extraEnv)) }} env: + {{- if $injectLocalEndpointHost }} + - name: RUSTFS_CHART_POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: RUSTFS_LOCAL_ENDPOINT_HOST + value: {{ printf "$(RUSTFS_CHART_POD_NAME).%s-headless.%s.svc.%s" (include "rustfs.fullname" $) $.Release.Namespace (include "rustfs.clusterDomain" $) | quote }} + {{- end }} + {{- with $.Values.extraEnv }} {{- toYaml . | nindent 12 }} + {{- end }} {{- end }} envFrom: - configMapRef: diff --git a/helm/rustfs/values.yaml b/helm/rustfs/values.yaml index 3ac64b9e3..7a254b869 100644 --- a/helm/rustfs/values.yaml +++ b/helm/rustfs/values.yaml @@ -30,11 +30,22 @@ drivesPerNode: null # Provide the DNS root only, without a `svc.` prefix or leading/trailing dots. clusterDomain: cluster.local -# Maximum time an init container waits for distributed peers to publish DNS -# and for earlier peers to start listening. The wait is shared across all -# peers, not applied once per peer. +# Deprecated and ignored. Retained so existing values files continue to +# render after peer startup coordination moved into RustFS. +# RUSTFS_COMPAT_TODO(rustfs-5416): Keep the removed init-gate setting visible to existing values files. Remove after the minimum supported direct-upgrade chart includes localEndpointHost.autoInject and no longer renders peer gates. startupWaitTimeoutSeconds: 300 +# Inject a pod-specific endpoint identity for chart-generated distributed +# topologies. The default enables injection unless secret.existingSecret is +# set, because the chart cannot inspect runtime overrides hidden in that +# Secret. Without injection, the server infers a unique matching domain +# endpoint from its kernel hostname in Kubernetes auto/orchestrated mode; custom +# aliases retain bounded legacy DNS locality when no hostname matches. Configure +# RUSTFS_LOCAL_ENDPOINT_HOST explicitly for DNS-free startup. Set true for a +# credentials-only existing Secret or false to suppress chart injection. +localEndpointHost: + autoInject: null + # This sets the container image more information can be found here: https://kubernetes.io/docs/concepts/containers/images/ image: rustfs: # This sets the rustfs image repository and tag. @@ -223,7 +234,18 @@ config: default_key: "" -extraEnv: [] # This is for setting extra environment variables in the rustfs container. It should be a list of key value pairs. For example: +# Extra environment variables for the RustFS container. +# In distributed mode with chart-generated volumes (config.rustfs.volumes is +# empty), the chart injects a private Downward API pod name and +# RUSTFS_LOCAL_ENDPOINT_HOST to identify the pod's local endpoints. An explicit +# RUSTFS_LOCAL_ENDPOINT_HOST or RUSTFS_VOLUMES entry disables automatic anchor +# injection. An explicit bounded or fail-fast +# RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE, or an unrecognized static value, also keeps +# legacy DNS-based locality discovery. Otherwise Kubernetes auto-detection +# selects orchestrated startup when RustFS consumes the generated anchor. +# With custom volumes, the chart does not inject them and extraEnv may define +# the local endpoint identity explicitly. +extraEnv: [] # extraEnv: # - name: RUSTFS_ERASURE_SET_DRIVE_COUNT # value: "16" diff --git a/scripts/test_helm_templates.sh b/scripts/test_helm_templates.sh index 4bdccceb4..ecbaa4e83 100755 --- a/scripts/test_helm_templates.sh +++ b/scripts/test_helm_templates.sh @@ -36,6 +36,16 @@ render_distributed_statefulset() { ' } +statefulset_env_names() { + yq eval '[(.spec.template.spec.containers[0].env // [])[] | .name] | join(" ")' - +} + +statefulset_env_value() { + local rustfs_test_env_name=$1 + RUSTFS_TEST_ENV_NAME="$rustfs_test_env_name" \ + yq eval '.spec.template.spec.containers[0].env[] | select(.name == strenv(RUSTFS_TEST_ENV_NAME)) | .value' - +} + render_distributed_configmap() { helm template rustfs "$CHART_DIR" \ --namespace rustfs \ @@ -403,38 +413,308 @@ if grep -q "name: data$" <<<"$generic_eight_by_two"; then exit 1 fi -# Distributed startup coordination must use the configured endpoint port and -# stay transport-neutral so the same wait works with and without mTLS. -custom_port_startup=$(render_distributed_statefulset \ +# Issue #5416's exact three-node, one-drive topology must render all three +# ordinal hosts while each StatefulSet pod receives its own explicit anchor. +three_by_one_configmap=$(render_distributed_configmap --set replicaCount=3 --set drivesPerNode=1) +if ! grep -Fq 'RUSTFS_VOLUMES: "http://rustfs-{0...2}.rustfs-headless.rustfs.svc.cluster.local:9000/data"' <<<"$three_by_one_configmap"; then + echo "Three-node topology must render ordinals 0 through 2 in RUSTFS_VOLUMES" >&2 + exit 1 +fi +# The distributed init container only prepares directories. Peer DNS or TCP +# availability must not gate the RustFS process from applying its own quorum +# checks. The deprecated timeout remains accepted but is not rendered. +distributed_startup=$(render_distributed_statefulset \ + --set replicaCount=3 \ + --set drivesPerNode=1 \ + --set startupWaitTimeoutSeconds=0) +for removed_gate in nslookup 'nc -z' wait_for_peer STARTUP_WAIT_TIMEOUT_SECONDS ENDPOINT_PORT; do + if grep -Fq "$removed_gate" <<<"$distributed_startup"; then + echo "Distributed init must not contain the removed peer gate: $removed_gate" >&2 + exit 1 + fi +done +if ! grep -Fq 'mkdir -p /data/rustfs$i' <<<"$distributed_startup"; then + echo "Distributed init must keep per-drive directory initialization" >&2 + exit 1 +fi +if ! grep -Eq '^[[:space:]]+mkdir -p /data$' <<<"$distributed_startup"; then + echo "Distributed init must keep single-drive directory initialization" >&2 + exit 1 +fi + +# Chart-generated volumes use the Downward API pod name to construct the exact +# endpoint hostname. Order is load-bearing because Kubernetes only expands +# environment references that were defined earlier in the env list. +generated_env_names=$(statefulset_env_names <<<"$distributed_startup") +if [[ "$generated_env_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST" ]]; then + echo "Unexpected generated topology env order: $generated_env_names" >&2 + exit 1 +fi + +pod_name_field=$(yq eval '.spec.template.spec.containers[0].env[] | select(.name == "RUSTFS_CHART_POD_NAME") | .valueFrom.fieldRef.fieldPath' - <<<"$distributed_startup") +local_endpoint_host=$(statefulset_env_value RUSTFS_LOCAL_ENDPOINT_HOST <<<"$distributed_startup") +if [[ "$pod_name_field" != "metadata.name" ]]; then + echo "Generated topology must source RUSTFS_CHART_POD_NAME from the Downward API" >&2 + exit 1 +fi +if [[ "$local_endpoint_host" != '$(RUSTFS_CHART_POD_NAME).rustfs-headless.rustfs.svc.cluster.local' ]]; then + echo "Unexpected generated RUSTFS_LOCAL_ENDPOINT_HOST: $local_endpoint_host" >&2 + exit 1 +fi + +# Non-default release identity, namespace, cluster domain, mTLS, and endpoint +# port must compose into one generated topology without restoring peer gates. +nondefault_generated=$(helm template tenant "$CHART_DIR" \ + --namespace object-storage \ + --set secret.rustfs.access_key=test-access-key \ + --set secret.rustfs.secret_key=test-secret-key \ + --set clusterDomain=cluster.corp \ + --set mtls.enabled=true \ --set service.endpoint.port=9443 \ --set config.rustfs.address=:9443) -grep -q 'name: ENDPOINT_PORT' <<<"$custom_port_startup" -grep -A1 'name: ENDPOINT_PORT' <<<"$custom_port_startup" | grep -q 'value: "9443"' -grep -q 'nc -z -w 2' <<<"$custom_port_startup" -if grep -q 'http.*9000/health' <<<"$custom_port_startup"; then - echo "Distributed startup wait must not hardcode the HTTP scheme or port 9000" >&2 +nondefault_statefulset=$(awk ' + /^# Source: rustfs\/templates\/statefulset.yaml$/ { in_statefulset = 1 } + in_statefulset && /^---$/ { exit } + in_statefulset { print } +' <<<"$nondefault_generated") +nondefault_local_host=$(statefulset_env_value RUSTFS_LOCAL_ENDPOINT_HOST <<<"$nondefault_statefulset") +nondefault_namespace=$(yq eval '.metadata.namespace' - <<<"$nondefault_statefulset") +nondefault_endpoint_port=$(yq eval '.spec.template.spec.containers[0].ports[] | select(.name == "endpoint") | .containerPort' - <<<"$nondefault_statefulset") +mtls_liveness_command=$(yq eval '.spec.template.spec.containers[0].livenessProbe.exec.command[-1]' - <<<"$nondefault_statefulset") +mtls_readiness_command=$(yq eval '.spec.template.spec.containers[0].readinessProbe.exec.command[-1]' - <<<"$nondefault_statefulset") +if [[ "$nondefault_local_host" != '$(RUSTFS_CHART_POD_NAME).tenant-rustfs-headless.object-storage.svc.cluster.corp' ]]; then + echo "Combined generated topology produced an unexpected local endpoint host: $nondefault_local_host" >&2 + exit 1 +fi +if [[ "$nondefault_namespace" != "object-storage" || "$nondefault_endpoint_port" != "9443" ]]; then + echo "Combined generated topology must retain its namespace and custom endpoint port" >&2 + exit 1 +fi +if ! grep -Fq 'https://127.0.0.1:9443/health' <<<"$mtls_liveness_command" || + grep -Fq '/health/ready' <<<"$mtls_liveness_command"; then + echo "mTLS liveness must use the process health endpoint, not readiness" >&2 + exit 1 +fi +if ! grep -Fq 'https://127.0.0.1:9443/health/ready' <<<"$mtls_readiness_command"; then + echo "mTLS readiness must use the ready endpoint" >&2 + exit 1 +fi +if ! grep -Fq 'RUSTFS_VOLUMES: "https://tenant-rustfs-{0...3}.tenant-rustfs-headless.object-storage.svc.cluster.corp:9443/data/rustfs{0...3}"' <<<"$nondefault_generated"; then + echo "Combined generated topology must use its release fullname, namespace, domain, mTLS scheme, and endpoint port" >&2 + exit 1 +fi +if grep -Eq 'nslookup|nc -z|wait_for_peer' <<<"$nondefault_generated"; then + echo "Combined generated topology must not render peer DNS or TCP gates" >&2 exit 1 fi -mtls_startup=$(render_distributed_statefulset --set mtls.enabled=true) -grep -q 'nc -z -w 2' <<<"$mtls_startup" -if grep -q 'wget .*http.*health' <<<"$mtls_startup"; then - echo "mTLS startup wait must not use a plaintext HTTP health probe" >&2 +default_scheme_port_generated=$(render_distributed_configmap \ + --set service.endpoint.port=80 \ + --set config.rustfs.address=:80) +if ! grep -Fq 'rustfs-headless.rustfs.svc.cluster.local:80/' <<<"$default_scheme_port_generated"; then + echo "Chart-generated topology must retain an explicit HTTP default port" >&2 exit 1 fi -# The timeout is one global deadline and invalid non-positive values fail -# during rendering instead of creating an unbounded init wait. -grep -q 'deadline=$(($(date +%s) + STARTUP_WAIT_TIMEOUT_SECONDS))' <<<"$custom_port_startup" -invalid_startup_timeout_status=0 -render_distributed_statefulset --set startupWaitTimeoutSeconds=0 >/dev/null 2>&1 || invalid_startup_timeout_status=$? -if [[ $invalid_startup_timeout_status -eq 0 ]]; then - echo "Distributed mode with startupWaitTimeoutSeconds=0 must fail rendering" >&2 +# Unrelated extraEnv entries follow the generated entries so dependent +# environment expansion keeps working. +generated_with_extra_env=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=CUSTOM_ENV' \ + --set 'extraEnv[0].value=custom-value') +generated_with_extra_names=$(statefulset_env_names <<<"$generated_with_extra_env") +if [[ "$generated_with_extra_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST CUSTOM_ENV" ]]; then + echo "extraEnv must be appended after generated topology env entries" >&2 exit 1 fi -# Every pool waits for DNS across the complete normalized pool list, while -# port availability follows the stable pool-index/ordinal startup order. +# Explicit bounded/fail-fast modes retain their legacy DNS locality semantics, +# so the generated anchor must not make the runtime reject the configuration. +for legacy_wait_mode in bounded ' Strict '; do + bounded_wait_mode=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE' \ + --set-string "extraEnv[0].value=$legacy_wait_mode") + bounded_wait_env_names=$(statefulset_env_names <<<"$bounded_wait_mode") + bounded_wait_value=$(yq eval '.spec.template.spec.containers[0].env[0].value' - <<<"$bounded_wait_mode") + if [[ "$bounded_wait_env_names" != "RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE" || "$bounded_wait_value" != "$legacy_wait_mode" ]]; then + echo "Startup mode $legacy_wait_mode must opt out of the generated local endpoint anchor" >&2 + exit 1 + fi +done + +unknown_wait_mode=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE' \ + --set 'extraEnv[0].value=invalid-mode') +unknown_wait_env_names=$(statefulset_env_names <<<"$unknown_wait_mode") +if [[ "$unknown_wait_env_names" != "RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE" ]]; then + echo "An unknown explicit startup mode must not receive a generated local endpoint anchor" >&2 + exit 1 +fi + +dynamic_wait_mode=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE' \ + --set 'extraEnv[0].valueFrom.configMapKeyRef.name=startup-policy' \ + --set 'extraEnv[0].valueFrom.configMapKeyRef.key=mode') +dynamic_wait_env_names=$(statefulset_env_names <<<"$dynamic_wait_mode") +if [[ "$dynamic_wait_env_names" != "RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE" ]]; then + echo "Dynamically sourced startup mode must opt out of the generated local endpoint anchor" >&2 + exit 1 +fi + +orchestrated_wait_mode=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE' \ + --set 'extraEnv[0].value=orchestrated') +orchestrated_wait_env_names=$(statefulset_env_names <<<"$orchestrated_wait_mode") +if [[ "$orchestrated_wait_env_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST RUSTFS_STARTUP_TOPOLOGY_WAIT_MODE" ]]; then + echo "Explicit orchestrated startup mode must retain one generated local endpoint anchor" >&2 + exit 1 +fi + +# Existing POD_NAME remains independent from the chart-private Downward API +# variable, while an explicit anchor disables automatic anchor injection. +existing_pod_name=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=POD_NAME' \ + --set 'extraEnv[0].valueFrom.fieldRef.fieldPath=metadata.name') +existing_pod_name_names=$(statefulset_env_names <<<"$existing_pod_name") +existing_pod_name_count=$(yq eval '[.spec.template.spec.containers[0].env[] | select(.name == "POD_NAME")] | length' - <<<"$existing_pod_name") +existing_pod_name_field=$(yq eval '.spec.template.spec.containers[0].env[] | select(.name == "POD_NAME") | .valueFrom.fieldRef.fieldPath' - <<<"$existing_pod_name") +existing_chart_pod_name_field=$(yq eval '.spec.template.spec.containers[0].env[] | select(.name == "RUSTFS_CHART_POD_NAME") | .valueFrom.fieldRef.fieldPath' - <<<"$existing_pod_name") +existing_pod_name_anchor=$(statefulset_env_value RUSTFS_LOCAL_ENDPOINT_HOST <<<"$existing_pod_name") +if [[ "$existing_pod_name_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST POD_NAME" || + "$existing_pod_name_count" != "1" || + "$existing_pod_name_field" != "metadata.name" || + "$existing_chart_pod_name_field" != "metadata.name" || + "$existing_pod_name_anchor" != '$(RUSTFS_CHART_POD_NAME).rustfs-headless.rustfs.svc.cluster.local' ]]; then + echo "An existing extraEnv POD_NAME must be preserved without interfering with the automatic anchor" >&2 + exit 1 +fi + +existing_local_anchor=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_LOCAL_ENDPOINT_HOST' \ + --set-string 'extraEnv[0].value=rustfs-0.rustfs-headless.rustfs.svc.cluster.local') +existing_local_anchor_names=$(statefulset_env_names <<<"$existing_local_anchor") +existing_local_anchor_value=$(statefulset_env_value RUSTFS_LOCAL_ENDPOINT_HOST <<<"$existing_local_anchor") +if [[ "$existing_local_anchor_names" != "RUSTFS_LOCAL_ENDPOINT_HOST" || "$existing_local_anchor_value" != "rustfs-0.rustfs-headless.rustfs.svc.cluster.local" ]]; then + echo "An existing explicit local endpoint anchor must be preserved without a generated duplicate" >&2 + exit 1 +fi + +# `helm upgrade --reuse-values` can omit keys introduced by the new chart. +# A missing localEndpointHost map keeps the new safe default instead of +# failing template evaluation. +missing_local_endpoint_host_values=$(render_distributed_statefulset \ + --set-json 'localEndpointHost=null') +missing_local_endpoint_host_names=$(statefulset_env_names <<<"$missing_local_endpoint_host_values") +if [[ "$missing_local_endpoint_host_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST" ]]; then + echo "Missing localEndpointHost values from a reused release must default to automatic anchor injection" >&2 + exit 1 +fi + +# An opaque existing Secret keeps the historical DNS path by default because +# it may hide runtime topology or startup-mode overrides. A credentials-only +# Secret can explicitly opt in without changing envFrom precedence. +existing_secret_statefulset=$(render_distributed_statefulset \ + --set secret.existingSecret=legacy-rustfs-config) +existing_secret_env_names=$(statefulset_env_names <<<"$existing_secret_statefulset") +existing_secret_env_from=$(yq eval '[.spec.template.spec.containers[0].envFrom[] | (.configMapRef.name // .secretRef.name)] | join(" ")' - <<<"$existing_secret_statefulset") +if [[ -n "$existing_secret_env_names" || "$existing_secret_env_from" != "rustfs-config legacy-rustfs-config" ]]; then + echo "An opaque existingSecret must default to the legacy environment path" >&2 + exit 1 +fi + +existing_secret_opt_in=$(render_distributed_statefulset \ + --set secret.existingSecret=legacy-rustfs-config \ + --set localEndpointHost.autoInject=true) +existing_secret_opt_in_names=$(statefulset_env_names <<<"$existing_secret_opt_in") +existing_secret_opt_in_env_from=$(yq eval '[.spec.template.spec.containers[0].envFrom[] | (.configMapRef.name // .secretRef.name)] | join(" ")' - <<<"$existing_secret_opt_in") +if [[ "$existing_secret_opt_in_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST" || + "$existing_secret_opt_in_env_from" != "rustfs-config legacy-rustfs-config" ]]; then + echo "A credentials-only existingSecret must support explicit automatic anchor injection" >&2 + exit 1 +fi + +# A Secret that hides a custom topology can explicitly preserve the historical +# DNS path. No explicit env entries are added, and envFrom ordering is unchanged. +existing_secret_opt_out=$(render_distributed_statefulset \ + --set secret.existingSecret=legacy-rustfs-config \ + --set localEndpointHost.autoInject=false) +existing_secret_opt_out_env_names=$(statefulset_env_names <<<"$existing_secret_opt_out") +existing_secret_opt_out_env_from=$(yq eval '[.spec.template.spec.containers[0].envFrom[] | (.configMapRef.name // .secretRef.name)] | join(" ")' - <<<"$existing_secret_opt_out") +if [[ -n "$existing_secret_opt_out_env_names" || "$existing_secret_opt_out_env_from" != "rustfs-config legacy-rustfs-config" ]]; then + echo "localEndpointHost.autoInject=false must preserve the legacy existingSecret environment path" >&2 + exit 1 +fi + +quoted_local_endpoint_host_status=0 +quoted_local_endpoint_host_error=$(render_distributed_statefulset \ + --set secret.existingSecret=legacy-rustfs-config \ + --set-string localEndpointHost.autoInject=false 2>&1) || quoted_local_endpoint_host_status=$? +if [[ $quoted_local_endpoint_host_status -eq 0 || + "$quoted_local_endpoint_host_error" != *"localEndpointHost.autoInject must be a boolean or null"* ]]; then + echo "A quoted localEndpointHost.autoInject value must fail closed instead of becoming truthy" >&2 + exit 1 +fi + +missing_local_endpoint_host_with_secret=$(render_distributed_statefulset \ + --set secret.existingSecret=legacy-rustfs-config \ + --set-json 'localEndpointHost=null') +if [[ -n "$(statefulset_env_names <<<"$missing_local_endpoint_host_with_secret")" ]]; then + echo "A reused release without localEndpointHost values must not override an opaque existingSecret" >&2 + exit 1 +fi + +# An explicit topology disables inference, but an address override alone keeps +# the generated anchor so RustFS can validate the effective port before format +# initialization. +volumes_override_statefulset=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_VOLUMES' \ + --set-string 'extraEnv[0].value=http://legacy.example/data') +volumes_override_names=$(statefulset_env_names <<<"$volumes_override_statefulset") +volumes_override_value=$(yq eval '.spec.template.spec.containers[0].env[0].value' - <<<"$volumes_override_statefulset") +if [[ "$volumes_override_names" != "RUSTFS_VOLUMES" || "$volumes_override_value" != "http://legacy.example/data" ]]; then + echo "extraEnv RUSTFS_VOLUMES must retain precedence without a generated anchor" >&2 + exit 1 +fi + +address_override_statefulset=$(render_distributed_statefulset \ + --set 'extraEnv[0].name=RUSTFS_ADDRESS' \ + --set-string 'extraEnv[0].value=:9443') +address_override_names=$(statefulset_env_names <<<"$address_override_statefulset") +address_override_value=$(statefulset_env_value RUSTFS_ADDRESS <<<"$address_override_statefulset") +address_override_anchor_count=$(yq eval '[.spec.template.spec.containers[0].env[] | select(.name == "RUSTFS_LOCAL_ENDPOINT_HOST")] | length' - <<<"$address_override_statefulset") +if [[ "$address_override_names" != "RUSTFS_CHART_POD_NAME RUSTFS_LOCAL_ENDPOINT_HOST RUSTFS_ADDRESS" || + "$address_override_value" != ":9443" || + "$address_override_anchor_count" != "1" ]]; then + echo "extraEnv RUSTFS_ADDRESS must retain one generated local endpoint anchor" >&2 + exit 1 +fi + +# Explicit volumes remain authoritative: the chart must not infer any identity, +# while a user-supplied local endpoint anchor remains allowed. +custom_volumes_statefulset=$(render_distributed_statefulset \ + --set config.rustfs.volumes=http://example.test/data) +custom_volumes_env_names=$(statefulset_env_names <<<"$custom_volumes_statefulset") +if [[ -n "$custom_volumes_env_names" ]]; then + echo "Custom volumes must not receive generated topology env entries" >&2 + exit 1 +fi + +custom_volumes_with_anchor=$(render_distributed_statefulset \ + --set config.rustfs.volumes=http://example.test/data \ + --set 'extraEnv[0].name=RUSTFS_LOCAL_ENDPOINT_HOST' \ + --set 'extraEnv[0].value=example.test') +custom_anchor=$(statefulset_env_value RUSTFS_LOCAL_ENDPOINT_HOST <<<"$custom_volumes_with_anchor") +if [[ "$custom_anchor" != "example.test" ]]; then + echo "Custom volumes must allow a user-supplied local endpoint anchor" >&2 + exit 1 +fi + +if grep -q 'RUSTFS_LOCAL_ENDPOINT_HOST' <<<"$standalone_default_output"; then + echo "Standalone mode must not receive a distributed local endpoint anchor" >&2 + exit 1 +fi + +# Every generated server pool uses its runtime pod name with the shared root +# headless service. No pool waits for another pool's DNS or TCP listener. multi_pool_startup=$(helm template rustfs "$CHART_DIR" \ --namespace rustfs \ --set secret.rustfs.access_key=test-access-key \ @@ -442,14 +722,19 @@ multi_pool_startup=$(helm template rustfs "$CHART_DIR" \ --set pools.enabled=true \ --set 'pools.list[0].replicaCount=2' \ --set 'pools.list[1].replicaCount=2') -grep -q 'nslookup "rustfs-0.rustfs-headless.rustfs.svc.cluster.local"' <<<"$multi_pool_startup" -grep -q 'nslookup "rustfs-pool1-1.rustfs-headless.rustfs.svc.cluster.local"' <<<"$multi_pool_startup" -if grep -q 'rustfs-pool1-headless' <<<"$multi_pool_startup"; then - echo "Multi-pool startup wait must use the shared root headless service" >&2 +multi_pool_anchor_count=$(grep -c 'name: RUSTFS_LOCAL_ENDPOINT_HOST' <<<"$multi_pool_startup" || true) +if [[ $multi_pool_anchor_count -ne 2 ]]; then + echo "Every generated server pool must receive RUSTFS_LOCAL_ENDPOINT_HOST" >&2 + exit 1 +fi +if grep -Eq 'nslookup|nc -z|wait_for_peer' <<<"$multi_pool_startup"; then + echo "Multi-pool init must not wait for peer DNS or TCP" >&2 + exit 1 +fi +if grep -q 'rustfs-pool1-headless' <<<"$multi_pool_startup"; then + echo "Multi-pool local endpoint identity must use the shared root headless service" >&2 exit 1 fi -grep -q 'wait_for_peer "rustfs-1:${ENDPOINT_PORT}"' <<<"$multi_pool_startup" -grep -q 'wait_for_peer "rustfs-pool1-${i}:${ENDPOINT_PORT}"' <<<"$multi_pool_startup" # volumeClaimTemplates must not contain empty annotations when pvcAnnotations are unset, # because Kubernetes treats annotations: {} as a mutation of the immutable field.