From 5c99ca132815519efcfaa87a0d749297a24f1457 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sun, 26 Jul 2026 08:28:38 +0800 Subject: [PATCH] fix(ecstore): fail fast on missing RPC secret in distributed startup (#5248) When endpoints include remote nodes and no internode RPC secret is resolvable, every remote format read fails client-side with "No valid auth token" and startup dies ~2 minutes later with the misleading "store init failed to load formats after 10 retries: erasure read quorum" error (issues #4939, #5153). Add a preflight to ECStore init that resolves the RPC secret once before any disk opens: if any endpoint is non-local and resolution fails, abort immediately with the operator-facing remediation message. The preflight also emits the "RPC auth secret resolution failed" log line so the log-analyzer rpc-secret-resolution rule keeps firing for this scenario. Single-node (all-local) topologies never invoke the resolver. --- crates/ecstore/src/store/init.rs | 170 +++++++++++++++++- .../internode-grpc-benchmark-runbook.md | 6 +- 2 files changed, 171 insertions(+), 5 deletions(-) diff --git a/crates/ecstore/src/store/init.rs b/crates/ecstore/src/store/init.rs index bc566d855..985f24cb2 100644 --- a/crates/ecstore/src/store/init.rs +++ b/crates/ecstore/src/store/init.rs @@ -19,6 +19,7 @@ use crate::runtime::instance::InstanceContext; use crate::runtime::sources as runtime_sources; use crate::storage_api_contracts::object::EcstoreObjectIO; use rustfs_config::server_config::KVS; +use rustfs_credentials::{RPC_SECRET_REQUIRED_OPERATOR_MESSAGE, try_get_rpc_token}; use tracing::{debug, error, info, warn}; const LOG_COMPONENT_ECSTORE: &str = "ecstore"; @@ -27,6 +28,7 @@ const EVENT_DECOMMISSION_RESUME_RETRY: &str = "decommission_resume_retry"; const EVENT_DECOMMISSION_RESUME_FAILED: &str = "decommission_resume_failed"; const EVENT_STORE_FORMAT_RETRY: &str = "store_format_retry"; const EVENT_ECSTORE_INIT_STATUS: &str = "ecstore_init_status"; +const EVENT_STORE_RPC_SECRET_PREFLIGHT_FAILED: &str = "store_rpc_secret_preflight_failed"; fn pool_first_endpoint_is_local(pool: &crate::layout::endpoints::PoolEndpoints) -> bool { pool.endpoints.as_ref().first().is_some_and(|endpoint| endpoint.is_local) @@ -49,6 +51,48 @@ fn resolve_startup_pool_defaults_with( drive_counts.into_iter().map(ec_drives_no_config).collect() } +/// Fail fast when the topology spans remote nodes but no internode RPC secret +/// resolves. Every remote format read would otherwise fail client-side with +/// "No valid auth token" and startup would retry for minutes before dying with +/// a misleading "erasure read quorum" error (issues #4939, #5153). +fn preflight_startup_rpc_secret(endpoint_pools: &EndpointServerPools) -> Result<()> { + preflight_startup_rpc_secret_with(endpoint_pools, try_get_rpc_token) +} + +fn preflight_startup_rpc_secret_with( + endpoint_pools: &EndpointServerPools, + resolve_rpc_token: impl FnOnce() -> std::io::Result, +) -> Result<()> { + let has_remote_endpoint = endpoint_pools + .as_ref() + .iter() + .flat_map(|pool| pool.endpoints.as_ref().iter()) + .any(|endpoint| !endpoint.is_local); + + if !has_remote_endpoint { + return Ok(()); + } + + match resolve_rpc_token() { + Ok(_) => Ok(()), + Err(err) => { + // The message prefix must stay aligned with the runtime log in + // cluster/rpc/http_auth.rs: the log-analyzer `rpc-secret-resolution` + // rule anchors on it, and this preflight aborts before that log + // would ever be emitted. + error!( + event = EVENT_STORE_RPC_SECRET_PREFLIGHT_FAILED, + component = LOG_COMPONENT_ECSTORE, + subsystem = LOG_SUBSYSTEM_STORE_INIT, + "RPC auth secret resolution failed: {err}; {RPC_SECRET_REQUIRED_OPERATOR_MESSAGE}" + ); + Err(Error::other(format!( + "store init aborted: endpoints include remote nodes but {err}; {RPC_SECRET_REQUIRED_OPERATOR_MESSAGE}" + ))) + } + } +} + fn should_resume_local_decommission(endpoints: &EndpointServerPools, idx: usize) -> Result { let pool = endpoints.as_ref().get(idx).ok_or_else(|| { Error::other(format!( @@ -218,6 +262,8 @@ impl ECStore { // from config before the store is marked ready. let default_pool_parities = resolve_startup_pool_defaults(&endpoint_pools)?; + preflight_startup_rpc_secret(&endpoint_pools)?; + let mut deployment_id = None; // let (endpoint_pools, _) = EndpointServerPools::create_server_endpoints(address.as_str(), &layouts)?; @@ -522,9 +568,10 @@ impl ECStore { mod tests { use super::{ LOCAL_DECOMMISSION_RESUME_MAX_CONFIG_RETRIES, load_pool_meta_for_startup, pool_first_endpoint_is_local, - resolve_startup_pool_defaults_with, resolve_store_init_stage_result, save_validated_pool_meta_for_startup, - should_auto_start_rebalance_after_init, should_auto_start_rebalance_after_recovered_meta, - should_resume_local_decommission, should_retry_local_decommission_resume, wait_for_local_decommission_resume_delay, + 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_auto_start_rebalance_after_recovered_meta, should_resume_local_decommission, + should_retry_local_decommission_resume, wait_for_local_decommission_resume_delay, }; #[cfg(feature = "test-util")] use crate::{ @@ -854,6 +901,123 @@ mod tests { ); } + fn rpc_preflight_pools(pools: Vec>) -> EndpointServerPools { + EndpointServerPools::from( + pools + .into_iter() + .enumerate() + .map(|(pool_index, endpoints)| PoolEndpoints { + legacy: false, + set_count: 1, + drives_per_set: endpoints.len(), + endpoints: Endpoints::from(endpoints), + cmd_line: format!("pool-{pool_index}"), + platform: String::new(), + }) + .collect::>(), + ) + } + + fn parsed_local_endpoint() -> Endpoint { + let mut endpoint = Endpoint::try_from("http://127.0.0.1:9000/data").expect("local endpoint should parse"); + endpoint.is_local = true; + endpoint + } + + fn parsed_remote_endpoint() -> Endpoint { + let mut endpoint = Endpoint::try_from("http://10.0.0.2:9000/data").expect("remote endpoint should parse"); + endpoint.is_local = false; + endpoint + } + + #[test] + fn test_rpc_secret_preflight_aborts_for_remote_endpoints_without_secret() { + // The remote endpoint intentionally sits behind a local one: the + // preflight must scan every endpoint, not just the first. + let pools = rpc_preflight_pools(vec![vec![parsed_local_endpoint(), parsed_remote_endpoint()]]); + + let err = preflight_startup_rpc_secret_with(&pools, || { + Err(std::io::Error::other(rustfs_credentials::RPC_SECRET_REQUIRED_MESSAGE)) + }) + .expect_err("remote endpoints without an RPC secret must abort startup before the format retry loop"); + + let message = err.to_string(); + assert!(message.contains("store init aborted"), "unexpected error: {message}"); + assert!( + message.contains(rustfs_credentials::RPC_SECRET_REQUIRED_MESSAGE), + "error must state the resolution failure: {message}" + ); + assert!( + message.contains(rustfs_credentials::RPC_SECRET_REQUIRED_OPERATOR_MESSAGE), + "error must carry the operator remediation guidance: {message}" + ); + } + + #[test] + fn test_rpc_secret_preflight_aborts_for_remote_endpoint_in_later_pool() { + // The remote endpoint hides in a later, otherwise-local pool: a + // first-pool shortcut (the `first_local` shape) must not pass. + let pools = rpc_preflight_pools(vec![vec![parsed_local_endpoint()], vec![parsed_remote_endpoint()]]); + + let err = preflight_startup_rpc_secret_with(&pools, || { + Err(std::io::Error::other(rustfs_credentials::RPC_SECRET_REQUIRED_MESSAGE)) + }) + .expect_err("a remote endpoint in a later pool must abort startup without an RPC secret"); + + let message = err.to_string(); + assert!(message.contains("store init aborted"), "unexpected error: {message}"); + } + + #[test] + fn test_rpc_secret_preflight_skips_resolution_for_local_only_endpoints() { + preflight_startup_rpc_secret_with(&rpc_preflight_pools(vec![vec![parsed_local_endpoint()]]), || { + panic!("single-node topology must not resolve an RPC secret") + }) + .expect("local-only topology must start without an RPC secret"); + } + + #[test] + fn test_rpc_secret_preflight_accepts_remote_endpoints_with_resolved_secret() { + preflight_startup_rpc_secret_with(&rpc_preflight_pools(vec![vec![parsed_remote_endpoint()]]), || { + Ok("resolved-secret".to_string()) + }) + .expect("a resolvable RPC secret must not block startup"); + } + + #[tokio::test] + #[serial_test::serial(storage_class_env)] + async fn test_new_with_instance_ctx_aborts_before_format_retry_loop_without_rpc_secret() { + // Guard: under a shared-process `cargo test` run another test may have + // already pinned the process-global RPC secret (ensure_test_rpc_secret), + // which would let init proceed past the preflight into remote disk + // init. A failing probe pins nothing, so it cannot poison later tests; + // under nextest's process-per-test model (CI) the guard never fires. + if rustfs_credentials::try_get_rpc_token().is_ok() { + return; + } + + let mut remote_endpoint = parsed_remote_endpoint(); + remote_endpoint.set_pool_index(0); + remote_endpoint.set_set_index(0); + remote_endpoint.set_disk_index(0); + + let err = crate::store::ECStore::new_with_instance_ctx( + "127.0.0.1:0".parse().expect("test address"), + rpc_preflight_pools(vec![vec![remote_endpoint]]), + CancellationToken::new(), + Arc::new(crate::runtime::instance::InstanceContext::new()), + ) + .await + .expect_err("distributed init without an RPC secret must abort before the format retry loop"); + + let message = err.to_string(); + assert!(message.contains("store init aborted"), "unexpected error: {message}"); + assert!( + message.contains(rustfs_credentials::RPC_SECRET_REQUIRED_OPERATOR_MESSAGE), + "abort must carry the operator remediation guidance: {message}" + ); + } + fn endpoint_pools_with_drive_counts(counts: &[usize]) -> EndpointServerPools { EndpointServerPools::from( counts diff --git a/docs/operations/internode-grpc-benchmark-runbook.md b/docs/operations/internode-grpc-benchmark-runbook.md index bee4d3d6d..298863342 100644 --- a/docs/operations/internode-grpc-benchmark-runbook.md +++ b/docs/operations/internode-grpc-benchmark-runbook.md @@ -123,8 +123,10 @@ target/bench/internode-transport/ Captured while running the first real A/B on a 4-node ansible cluster; needed before any live run: - **RPC secret is mandatory on current `main`.** Internode RPC fails closed: default creds - (`RUSTFS_SECRET_KEY=rustfsadmin`) with no `RUSTFS_RPC_SECRET` → `No valid auth token` → the cluster - never reaches `storage_quorum`. Set a **non-default** `RUSTFS_RPC_SECRET`, identical on every node. + (`RUSTFS_SECRET_KEY=rustfsadmin`) with no `RUSTFS_RPC_SECRET` → the node aborts startup immediately + with `store init aborted: endpoints include remote nodes but RPC authentication secret is not + configured` (builds before the preflight instead showed `No valid auth token` and never reached + `storage_quorum`). Set a **non-default** `RUSTFS_RPC_SECRET`, identical on every node. - **systemd start timeout.** The install unit is `Type=notify`; READY only fires after quorum, which on freshly-purged disks exceeds a 30 s `TimeoutStartSec` → crash loop. Use a drop-in `TimeoutStartSec=infinity`. - **Server-side metrics need OTLP.** RustFS has no Prometheus pull endpoint (`/admin/v3/metrics` is NDJSON,