From 3f3e3f4f0513a48c01a4e187463120c948f4ada2 Mon Sep 17 00:00:00 2001 From: houseme Date: Mon, 17 Aug 2026 10:45:24 +0800 Subject: [PATCH 01/10] perf(get): avoid memory body stream wrapper (#6163) Use MemoryTrackedBytesStream directly as an s3s ByteStream so in-memory GET bodies avoid the generic StreamingBlob::wrap adapter while preserving exact remaining length, request lifecycle tracking, and length-mismatch failure semantics. Co-authored-by: heihutu --- rustfs/src/app/object_usecase.rs | 39 ++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 913d6eb52..643b4fb6e 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -1074,7 +1074,7 @@ where } impl futures::Stream for MemoryTrackedBytesStream { - type Item = std::io::Result; + type Item = Result; fn poll_next(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { let this = self.get_mut(); @@ -1105,7 +1105,8 @@ impl futures::Stream for MemoryTrackedBytesStream { return Poll::Ready(Some(Err(std::io::Error::new( std::io::ErrorKind::InvalidData, format!("materialized GET body length mismatch: expected {}, got {}", this.expected, actual), - )))); + ) + .into()))); } let Some(bytes) = this.bytes.take() else { @@ -1132,6 +1133,16 @@ impl futures::Stream for MemoryTrackedBytesStream { } } +impl ByteStream for MemoryTrackedBytesStream { + fn remaining_length(&self) -> RemainingLength { + if self.emitted || self.bytes.is_none() { + RemainingLength::new_exact(0) + } else { + RemainingLength::new_exact(self.expected) + } + } +} + impl Drop for MemoryTrackedBytesStream { fn drop(&mut self) { if self.lifecycle.is_finished() { @@ -4149,7 +4160,7 @@ impl DefaultObjectUsecase { let bytes_len = bytes.len(); let guard = rustfs_io_metrics::track_get_object_buffered_bytes(bytes_len); let remaining = usize::try_from(response_content_length.max(0)).unwrap_or(usize::MAX); - let blob = StreamingBlob::wrap(MemoryTrackedBytesStream::new(bytes, remaining, source, guard, lifecycle)); + let blob = StreamingBlob::new(MemoryTrackedBytesStream::new(bytes, remaining, source, guard, lifecycle)); if let Some(handoff_start) = handoff_start { rustfs_io_metrics::record_get_object_response_handoff( "single_chunk", @@ -12882,7 +12893,10 @@ mod tests { .await .expect("mismatched memory body must yield an item") .expect_err("a short memory body must fail the stream instead of serving a truncated body"); - assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + assert_eq!( + err.downcast_ref::().map(std::io::Error::kind), + Some(std::io::ErrorKind::InvalidData) + ); assert!(stream.next().await.is_none(), "stream must terminate after the error"); } @@ -12901,7 +12915,22 @@ mod tests { .await .expect("mismatched memory body must yield an item") .expect_err("an over-long memory body must fail the stream instead of serving mismatched bytes"); - assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + assert_eq!( + err.downcast_ref::().map(std::io::Error::kind), + Some(std::io::ErrorKind::InvalidData) + ); + } + + #[test] + fn memory_blob_preserves_exact_remaining_length() { + let blob = DefaultObjectUsecase::build_memory_bytes_blob( + Bytes::from_static(b"hello"), + 5, + GET_MEMORY_BODY_SOURCE_BUFFERED_BODY, + GetObjectBodyLifecycle::disabled(), + ); + + assert_eq!(blob.remaining_length().exact(), Some(5)); } #[tokio::test] From 7db38827776f2efb4a879a192242b35aa8a30067 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Mon, 17 Aug 2026 11:32:23 +0800 Subject: [PATCH 02/10] chore(obs): adjudicate 19 bare dead_code allows (#6162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backlog#1823 step 10, batch 2. Eighteen of the nineteen suppress nothing and are deleted; one was real and keeps an allow that now says why. Rotation::Never is constructed only by the rolling-appender tests at rolling.rs:456, 477 and 498, so the lib target reports it as never constructed. Its allow is restored with that reason. Finding it corrected the method used for batch 1. Removing all nineteen and running cargo check -p rustfs-obs --tests reported zero warnings even after touching every source file, while clippy --lib --tests -D warnings caught Rotation::Never. cargo's warning output is not a reliable completeness check — it does not re-emit for cached compilations, and touching the sources did not cover the lib target here. Later batches should treat clippy -D warnings as the gate; batch 1's six crates were re-checked under clippy and are clean. Taken with #6086, which cleared this crate's 44 module-level blankets and left six real items, obs has now had 63 dead-code suppressions examined, of which seven were suppressing anything at all. The rest sat on items that are publicly reachable, where dead_code never applied — the same shape as the swift module and kms's dek.rs. Verification: clippy --lib --tests -D warnings clean in the default, gpu and pyroscope lanes; cargo nextest run -p rustfs-obs 324 passed; make pre-commit exit 0. Ref rustfs/backlog#1823 (step 10). --- crates/obs/src/metrics/config.rs | 1 - crates/obs/src/metrics/report.rs | 3 --- crates/obs/src/metrics/schema/entry/descriptor.rs | 3 --- crates/obs/src/metrics/schema/entry/metric_name.rs | 2 -- crates/obs/src/metrics/schema/entry/metric_type.rs | 3 --- crates/obs/src/metrics/schema/entry/mod.rs | 1 - crates/obs/src/metrics/schema/entry/namespace.rs | 1 - crates/obs/src/metrics/schema/entry/path_utils.rs | 1 - crates/obs/src/metrics/schema/entry/subsystem.rs | 3 --- crates/obs/src/telemetry/rolling.rs | 5 ++++- 10 files changed, 4 insertions(+), 19 deletions(-) diff --git a/crates/obs/src/metrics/config.rs b/crates/obs/src/metrics/config.rs index 26b73d6b1..c43e0c1e1 100644 --- a/crates/obs/src/metrics/config.rs +++ b/crates/obs/src/metrics/config.rs @@ -17,7 +17,6 @@ use std::time::Duration; /// Environment variable key for the global default metrics interval (seconds). pub const ENV_DEFAULT_METRICS_INTERVAL: &str = "RUSTFS_METRICS_DEFAULT_INTERVAL_SEC"; /// Default interval for metrics collection if not specified otherwise. -#[allow(dead_code)] pub const DEFAULT_METRICS_INTERVAL: Duration = Duration::from_secs(60); /// Environment variable key for cluster metrics interval (seconds). diff --git a/crates/obs/src/metrics/report.rs b/crates/obs/src/metrics/report.rs index b03630272..b3a421f55 100644 --- a/crates/obs/src/metrics/report.rs +++ b/crates/obs/src/metrics/report.rs @@ -145,21 +145,18 @@ impl PrometheusMetric { } #[inline] - #[allow(dead_code)] pub fn with_label(mut self, key: &'static str, value: impl Into>) -> Self { self.labels.push((key, value.into())); self } #[inline] - #[allow(dead_code)] pub fn with_label_owned(mut self, key: &'static str, value: String) -> Self { self.labels.push((key, Cow::Owned(value))); self } #[inline] - #[allow(dead_code)] pub fn with_labels(mut self, labels: Vec<(&'static str, Cow<'static, str>)>) -> Self { self.labels = labels; self diff --git a/crates/obs/src/metrics/schema/entry/descriptor.rs b/crates/obs/src/metrics/schema/entry/descriptor.rs index c4612e1f8..fce80d998 100644 --- a/crates/obs/src/metrics/schema/entry/descriptor.rs +++ b/crates/obs/src/metrics/schema/entry/descriptor.rs @@ -16,7 +16,6 @@ use crate::{MetricName, MetricNamespace, MetricSubsystem, MetricType}; use std::collections::HashSet; /// MetricDescriptor - Metric descriptors -#[allow(dead_code)] #[derive(Debug, Clone)] pub struct MetricDescriptor { pub name: MetricName, @@ -52,7 +51,6 @@ impl MetricDescriptor { } /// Get the full metric name in Prometheus style: __ - #[allow(dead_code)] pub fn get_full_metric_name(&self) -> String { let namespace = self.namespace.as_str(); let formatted_subsystem = self.subsystem.as_str(); @@ -61,7 +59,6 @@ impl MetricDescriptor { } /// check whether the label is in the label set - #[allow(dead_code)] pub fn has_label(&mut self, label: &str) -> bool { self.get_label_set().contains(label) } diff --git a/crates/obs/src/metrics/schema/entry/metric_name.rs b/crates/obs/src/metrics/schema/entry/metric_name.rs index 7d22eaa62..d9ec407b3 100644 --- a/crates/obs/src/metrics/schema/entry/metric_name.rs +++ b/crates/obs/src/metrics/schema/entry/metric_name.rs @@ -13,7 +13,6 @@ // limitations under the License. /// The metric name is the individual name of the metric -#[allow(dead_code)] #[derive(Debug, Clone, PartialEq, Eq)] pub enum MetricName { // The generic metric name @@ -443,7 +442,6 @@ pub enum MetricName { } impl MetricName { - #[allow(dead_code)] pub fn as_str(&self) -> String { match self { Self::AuthTotal => "auth_total".to_string(), diff --git a/crates/obs/src/metrics/schema/entry/metric_type.rs b/crates/obs/src/metrics/schema/entry/metric_type.rs index 33d8a78dd..b16f4cd12 100644 --- a/crates/obs/src/metrics/schema/entry/metric_type.rs +++ b/crates/obs/src/metrics/schema/entry/metric_type.rs @@ -13,7 +13,6 @@ // limitations under the License. /// MetricType - Indicates the type of indicator -#[allow(dead_code)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MetricType { Counter, @@ -23,7 +22,6 @@ pub enum MetricType { impl MetricType { /// convert the metric type to a string representation - #[allow(dead_code)] pub fn as_str(&self) -> &'static str { match self { Self::Counter => "counter", @@ -34,7 +32,6 @@ impl MetricType { /// Convert the metric type to the Prometheus value type /// In a Rust implementation, this might return the corresponding Prometheus Rust client type - #[allow(dead_code)] pub fn as_prom(&self) -> &'static str { match self { Self::Counter => "counter.", diff --git a/crates/obs/src/metrics/schema/entry/mod.rs b/crates/obs/src/metrics/schema/entry/mod.rs index 87215d15c..c10b48b45 100644 --- a/crates/obs/src/metrics/schema/entry/mod.rs +++ b/crates/obs/src/metrics/schema/entry/mod.rs @@ -56,7 +56,6 @@ pub fn new_gauge_md( } /// create a new histogram indicator descriptor -#[allow(dead_code)] pub fn new_histogram_md( name: impl Into, help: impl Into, diff --git a/crates/obs/src/metrics/schema/entry/namespace.rs b/crates/obs/src/metrics/schema/entry/namespace.rs index 31c3ce590..fe9a5df1f 100644 --- a/crates/obs/src/metrics/schema/entry/namespace.rs +++ b/crates/obs/src/metrics/schema/entry/namespace.rs @@ -19,7 +19,6 @@ pub enum MetricNamespace { } impl MetricNamespace { - #[allow(dead_code)] pub fn as_str(&self) -> &'static str { match self { Self::RustFS => "rustfs", diff --git a/crates/obs/src/metrics/schema/entry/path_utils.rs b/crates/obs/src/metrics/schema/entry/path_utils.rs index f8b63da28..d2abc7bfd 100644 --- a/crates/obs/src/metrics/schema/entry/path_utils.rs +++ b/crates/obs/src/metrics/schema/entry/path_utils.rs @@ -14,7 +14,6 @@ /// Format the path to the metric name format /// Replace '/' and '-' with '_' -#[allow(dead_code)] pub fn format_path_to_metric_name(path: &str) -> String { path.trim_start_matches('/').replace(['/', '-'], "_") } diff --git a/crates/obs/src/metrics/schema/entry/subsystem.rs b/crates/obs/src/metrics/schema/entry/subsystem.rs index 1c313f6e8..7b0606890 100644 --- a/crates/obs/src/metrics/schema/entry/subsystem.rs +++ b/crates/obs/src/metrics/schema/entry/subsystem.rs @@ -102,7 +102,6 @@ impl MetricSubsystem { } /// Get the formatted metric name format string - #[allow(dead_code)] pub fn as_str(&self) -> String { format_path_to_metric_name(self.path()) } @@ -151,7 +150,6 @@ impl MetricSubsystem { } /// A convenient way to create custom subsystems directly - #[allow(dead_code)] pub fn new(path: impl Into) -> Self { Self::Custom(path.into()) } @@ -176,7 +174,6 @@ impl std::fmt::Display for MetricSubsystem { } } -#[allow(dead_code)] pub mod subsystems { use super::MetricSubsystem; diff --git a/crates/obs/src/telemetry/rolling.rs b/crates/obs/src/telemetry/rolling.rs index 3ff661bfd..c5680ac61 100644 --- a/crates/obs/src/telemetry/rolling.rs +++ b/crates/obs/src/telemetry/rolling.rs @@ -38,7 +38,10 @@ pub enum Rotation { Minutely, Hourly, Daily, - #[allow(dead_code)] + #[allow( + dead_code, + reason = "constructed only by this file's rolling-appender tests; the lib target cannot see them (backlog#1823)" + )] Never, } From a9691b67978c56cb38081f453fc4959e568275a6 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Mon, 17 Aug 2026 11:34:47 +0800 Subject: [PATCH 03/10] chore: adjudicate 19 bare dead_code allows across six leaf crates (#6161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backlog#1823 step 10, batch 1 of the repo-wide item-allow sweep. 227 bare #[allow(dead_code)] remain across 83 files; this takes the 19 in utils, notify, checksums, policy, keystone and trusted-proxies, which are small enough to verify end to end. Removing all 19 first, before writing any reason, matters: 8 of them suppress nothing. Every allow in utils, one in policy and three in notify sit on items that are publicly reachable, so dead_code never applied to them — the same shape as the swift module and kms's dek.rs. Writing a reason onto a no-op allow would dress noise up as considered judgement, so those are simply deleted. Three items are genuinely dead and go with their allows: notify's new_target_id_set, the AWS metadata fetcher's get_metadata_token, and policy's empty `pub struct Value;`, none of which is referenced anywhere in the tree. The remaining eight keep an allow, now saying why the item survives rather than who calls it. Two are exercised only by their own crate's tests (checksums' MD5_HEADER_NAME, policy's is_match_as_pattern_prefix). Four are fields written but never read back: keystone's verify_ssl, parsed from config after the reqwest client is already built; keystone's client handle, which keeps the Keystone client alive for the mapper's lifetime; the AWS IMDS endpoint, kept beside the client while requests build their own URLs; and notify's rules_map, whose own comment retains it for snapshot-time judgements no code performs. checksums' Md5 needed the most care. Crc32, Sha256 and seven others each have an arm in ChecksumAlgorithm::into_impl, and Md5 has none, which reads like a missing algorithm. It is not: ChecksumAlgorithm has no Md5 variant at all. S3 carries Content-MD5 as its own header, separate from the x-amz-checksum-* family, and this impl exists so both paths share the Checksum trait. The reason records that, so the next reader does not re-derive it. One measurement note for anyone continuing this sweep: cargo does not re-emit warnings for cached compilations, so a per-crate loop of `cargo check -p ` under-reports. checksums showed zero that way while actually carrying three. Touch the sources and check the crates in one invocation, then attribute by path. Verification: the six crates are warning-free under cargo check --tests; clippy --lib --tests -D warnings clean; cargo nextest run 1096 passed; make pre-commit exit 0. Ref rustfs/backlog#1823 (step 10). --- crates/checksums/src/http.rs | 5 +- crates/checksums/src/lib.rs | 10 +++- crates/keystone/src/client.rs | 5 +- crates/keystone/src/identity.rs | 5 +- crates/notify/src/rules/config.rs | 5 +- crates/notify/src/rules/rules_map.rs | 3 -- crates/notify/src/rules/target_id_set.rs | 6 --- crates/policy/src/policy/function.rs | 4 -- crates/policy/src/policy/utils/wildcard.rs | 6 ++- .../trusted-proxies/src/cloud/metadata/aws.rs | 53 ++----------------- crates/trusted-proxies/src/config/env.rs | 1 - crates/utils/src/io.rs | 1 - crates/utils/src/net.rs | 1 - crates/utils/src/os/fs_type.rs | 1 - crates/utils/src/path.rs | 1 - 15 files changed, 32 insertions(+), 75 deletions(-) diff --git a/crates/checksums/src/http.rs b/crates/checksums/src/http.rs index 1a369a42d..ef21bc3ba 100644 --- a/crates/checksums/src/http.rs +++ b/crates/checksums/src/http.rs @@ -38,7 +38,10 @@ pub const XXHASH_3_HEADER_NAME: &str = "x-amz-checksum-xxhash3"; pub const XXHASH_64_HEADER_NAME: &str = "x-amz-checksum-xxhash64"; pub const XXHASH_128_HEADER_NAME: &str = "x-amz-checksum-xxhash128"; -#[allow(dead_code)] +#[allow( + dead_code, + reason = "Content-MD5 wire name, resolved by header_name() below and asserted by this crate's tests (backlog#1823)" +)] pub(crate) static MD5_HEADER_NAME: &str = "content-md5"; pub const CHECKSUM_ALGORITHMS_IN_PRIORITY_ORDER: [&str; 5] = diff --git a/crates/checksums/src/lib.rs b/crates/checksums/src/lib.rs index a8da44545..5b566fe83 100644 --- a/crates/checksums/src/lib.rs +++ b/crates/checksums/src/lib.rs @@ -476,13 +476,19 @@ impl Checksum for Xxhash64 { } } -#[allow(dead_code)] #[derive(Debug, Default)] +#[allow( + dead_code, + reason = "Content-MD5 is not a ChecksumAlgorithm variant and has no arm in into_impl: S3 carries it as its own header, separate from the x-amz-checksum-* family. This impl exists so the two paths share the Checksum trait, and is asserted by this crate's tests (backlog#1823)" +)] struct Md5 { hasher: md5::Md5, } -#[allow(dead_code)] +#[allow( + dead_code, + reason = "Content-MD5 is not a ChecksumAlgorithm variant and has no arm in into_impl: S3 carries it as its own header, separate from the x-amz-checksum-* family. This impl exists so the two paths share the Checksum trait, and is asserted by this crate's tests (backlog#1823)" +)] impl Md5 { fn update(&mut self, bytes: &[u8]) { use md5::Digest; diff --git a/crates/keystone/src/client.rs b/crates/keystone/src/client.rs index d5df169e8..bd1d36d30 100644 --- a/crates/keystone/src/client.rs +++ b/crates/keystone/src/client.rs @@ -31,7 +31,10 @@ pub struct KeystoneClient { admin_password: Option, admin_project: Option, admin_domain: String, - #[allow(dead_code)] + #[allow( + dead_code, + reason = "TLS verification flag parsed from config; the reqwest client is built before it is consulted, so nothing reads it back (backlog#1823)" + )] verify_ssl: bool, /// Request timeout applied to the underlying HTTP client. timeout: std::time::Duration, diff --git a/crates/keystone/src/identity.rs b/crates/keystone/src/identity.rs index 96e45de17..f61ca5c2b 100644 --- a/crates/keystone/src/identity.rs +++ b/crates/keystone/src/identity.rs @@ -20,7 +20,10 @@ use tracing::{debug, info}; /// Maps Keystone identities to RustFS concepts pub struct KeystoneIdentityMapper { - #[allow(dead_code)] + #[allow( + dead_code, + reason = "keeps the Keystone client alive for the mapper's lifetime; the mapping paths do not call through it yet (backlog#1823)" + )] client: Arc, role_policy_map: HashMap, enable_tenant_prefix: bool, diff --git a/crates/notify/src/rules/config.rs b/crates/notify/src/rules/config.rs index 42ca136fa..624f99787 100644 --- a/crates/notify/src/rules/config.rs +++ b/crates/notify/src/rules/config.rs @@ -40,7 +40,10 @@ impl RuleEvents for RuleView { #[derive(Debug)] struct CompiledRules { // Keep RulesMap (can be used later if you want to make more complex judgments during the snapshot reading phase) - #[allow(dead_code)] + #[allow( + dead_code, + reason = "speculative retention: the comment above keeps it for richer snapshot-time judgements that no code performs yet (backlog#1823)" + )] rules_map: RulesMap, // for RulesContainer::iter_rules rule_views: Vec, diff --git a/crates/notify/src/rules/rules_map.rs b/crates/notify/src/rules/rules_map.rs index 9ae40ad85..7d1314893 100644 --- a/crates/notify/src/rules/rules_map.rs +++ b/crates/notify/src/rules/rules_map.rs @@ -187,7 +187,6 @@ impl RulesMap { /// # Parameters /// * `event_name` - The EventName from which to remove the rule. /// * `pattern` - The pattern of the rule to be removed. - #[allow(dead_code)] pub fn remove_rule(&mut self, event_name: &EventName, pattern: &str) { let mut remove_event = false; @@ -209,7 +208,6 @@ impl RulesMap { /// /// # Parameters /// * `event_names` - A slice of EventNames to be removed. - #[allow(dead_code)] pub fn remove_rules(&mut self, event_names: &[EventName]) { for event_name in event_names { self.map.remove(event_name); @@ -223,7 +221,6 @@ impl RulesMap { /// * `event_name` - The EventName to update. /// * `pattern` - The pattern of the rule to be updated. /// * `target_id` - The TargetID to be added. - #[allow(dead_code)] pub fn update_rule(&mut self, event_name: EventName, pattern: String, target_id: TargetID) { self.map.entry(event_name).or_default().add(pattern, target_id); self.total_events_mask |= event_name.mask(); // Update only the relevant bitmask diff --git a/crates/notify/src/rules/target_id_set.rs b/crates/notify/src/rules/target_id_set.rs index d5036975c..06c2c2f87 100644 --- a/crates/notify/src/rules/target_id_set.rs +++ b/crates/notify/src/rules/target_id_set.rs @@ -18,12 +18,6 @@ use rustfs_targets::arn::TargetID; /// TargetIDSet - A collection representation of TargetID. pub type TargetIdSet = HashSet; -/// Provides a Go-like method for TargetIdSet (can be implemented as trait if needed) -#[allow(dead_code)] -pub(crate) fn new_target_id_set(target_ids: Vec) -> TargetIdSet { - target_ids.into_iter().collect() -} - // HashSet has built-in clone, union, difference and other operations. // But the Go version of the method returns a new Set, and the HashSet method is usually iterator or modify itself. // If you need to exactly match Go's API style, you can add wrapper functions. diff --git a/crates/policy/src/policy/function.rs b/crates/policy/src/policy/function.rs index b1fa5d1ee..55bf7e13a 100644 --- a/crates/policy/src/policy/function.rs +++ b/crates/policy/src/policy/function.rs @@ -219,10 +219,6 @@ impl PartialEq for Functions { } } -#[derive(Clone, Serialize, Deserialize)] -#[allow(dead_code)] -pub struct Value; - #[cfg(test)] mod tests { use crate::policy::Functions; diff --git a/crates/policy/src/policy/utils/wildcard.rs b/crates/policy/src/policy/utils/wildcard.rs index 915cb153e..0b8fa3bb2 100644 --- a/crates/policy/src/policy/utils/wildcard.rs +++ b/crates/policy/src/policy/utils/wildcard.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#[allow(dead_code)] pub fn is_simple_match(pattern: P, name: N) -> bool where P: AsRef, @@ -29,7 +28,10 @@ where inner_match(pattern, name, false) } -#[allow(dead_code)] +#[allow( + dead_code, + reason = "prefix-matcher asserted by this file's tests; no production caller yet (backlog#1823)" +)] pub fn is_match_as_pattern_prefix(pattern: P, text: N) -> bool where P: AsRef, diff --git a/crates/trusted-proxies/src/cloud/metadata/aws.rs b/crates/trusted-proxies/src/cloud/metadata/aws.rs index 506eca925..c971cfa4d 100644 --- a/crates/trusted-proxies/src/cloud/metadata/aws.rs +++ b/crates/trusted-proxies/src/cloud/metadata/aws.rs @@ -27,6 +27,10 @@ use crate::CloudMetadataFetcher; #[derive(Debug, Clone)] pub struct AwsMetadataFetcher { client: Client, + #[allow( + dead_code, + reason = "IMDS endpoint retained beside the client it configures; requests build their own URLs (backlog#1823)" + )] metadata_endpoint: String, } @@ -46,55 +50,6 @@ impl AwsMetadataFetcher { metadata_endpoint: "http://169.254.169.254".to_string(), } } - - /// Retrieves an IMDSv2 token for secure metadata access. - #[allow(dead_code)] - async fn get_metadata_token(&self) -> Result { - let url = format!("{}/latest/api/token", self.metadata_endpoint); - - match self - .client - .put(&url) - .header("X-aws-ec2-metadata-token-ttl-seconds", "21600") - .send() - .await - { - Ok(response) => { - if response.status().is_success() { - let token = response - .text() - .await - .map_err(|e| AppError::cloud(format!("Failed to read IMDSv2 token: {}", e)))?; - Ok(token) - } else { - debug!( - event = "trusted_proxies.cloud_metadata", - component = "trusted_proxies", - subsystem = "aws_metadata", - provider = "aws", - operation = "imdsv2_token", - result = "http_error", - status = %response.status(), - "trusted proxy cloud metadata request failed" - ); - Err(AppError::cloud("Failed to obtain IMDSv2 token")) - } - } - Err(e) => { - debug!( - event = "trusted_proxies.cloud_metadata", - component = "trusted_proxies", - subsystem = "aws_metadata", - provider = "aws", - operation = "imdsv2_token", - result = "request_failed", - error = %e, - "trusted proxy cloud metadata request failed" - ); - Err(AppError::cloud(format!("IMDSv2 request failed: {}", e))) - } - } - } } #[async_trait] diff --git a/crates/trusted-proxies/src/config/env.rs b/crates/trusted-proxies/src/config/env.rs index a982ae0d8..2a8a3ff6a 100644 --- a/crates/trusted-proxies/src/config/env.rs +++ b/crates/trusted-proxies/src/config/env.rs @@ -68,7 +68,6 @@ pub fn is_env_set(key: &str) -> bool { } /// Returns a list of all proxy-related environment variables and their current values. -#[allow(dead_code)] pub fn get_all_proxy_env_vars() -> Vec<(String, String)> { let vars = [ ENV_TRUSTED_PROXY_ENABLED, diff --git a/crates/utils/src/io.rs b/crates/utils/src/io.rs index 92e69e5db..44388b0f5 100644 --- a/crates/utils/src/io.rs +++ b/crates/utils/src/io.rs @@ -68,7 +68,6 @@ pub async fn read_full_or_eof( /// Read exactly buf.len() bytes into buf, or return an error if EOF is reached before any bytes are read. /// Like Go's io.ReadFull. -#[allow(dead_code)] pub async fn read_full(reader: R, buf: &mut [u8]) -> std::io::Result { match read_full_or_eof(reader, buf).await? { Some(n) => Ok(n), diff --git a/crates/utils/src/net.rs b/crates/utils/src/net.rs index 873a6d940..3489ba41c 100644 --- a/crates/utils/src/net.rs +++ b/crates/utils/src/net.rs @@ -431,7 +431,6 @@ pub fn parse_and_resolve_address(addr_str: &str) -> std::io::Result Ok(resolved_addr) } -#[allow(dead_code)] pub fn bytes_stream(stream: S, content_length: usize) -> impl Stream> + Send + 'static where S: Stream> + Send + 'static, diff --git a/crates/utils/src/os/fs_type.rs b/crates/utils/src/os/fs_type.rs index a3ae1045c..3650cb793 100644 --- a/crates/utils/src/os/fs_type.rs +++ b/crates/utils/src/os/fs_type.rs @@ -16,7 +16,6 @@ /// /// The table follows Linux `include/uapi/linux/magic.h`; filesystem magic /// values without a stable Linux uapi source stay `UNKNOWN`. -#[allow(dead_code)] pub(crate) fn get_fs_type(fs_type: u64) -> &'static str { // Magic numbers for various filesystems. match fs_type { diff --git a/crates/utils/src/path.rs b/crates/utils/src/path.rs index c383dc822..8b53e764b 100644 --- a/crates/utils/src/path.rs +++ b/crates/utils/src/path.rs @@ -70,7 +70,6 @@ pub fn is_dir_object(object: &str) -> bool { /// /// If the object name ends with `GLOBAL_DIR_SUFFIX`, it is replaced with a slash. /// Otherwise, the name is returned as is. -#[allow(dead_code)] pub fn decode_dir_object(object: &str) -> String { if has_suffix(object, GLOBAL_DIR_SUFFIX) { format!("{}{}", object.trim_end_matches(GLOBAL_DIR_SUFFIX), SLASH_SEPARATOR) From c04ee41cf088b86150b6bb75f13fed8913fcf0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Mon, 17 Aug 2026 14:12:04 +0800 Subject: [PATCH 04/10] feat(site-replication): drain the retry queue from the reconcile tick (#6131) --- crates/madmin/src/site_replication.rs | 6 +- rustfs/src/admin/handlers/site_replication.rs | 1057 ++++++++++++++++- 2 files changed, 1051 insertions(+), 12 deletions(-) diff --git a/crates/madmin/src/site_replication.rs b/crates/madmin/src/site_replication.rs index a8813f99c..740924405 100644 --- a/crates/madmin/src/site_replication.rs +++ b/crates/madmin/src/site_replication.rs @@ -258,7 +258,7 @@ pub struct SRLDAPUser { pub api_version: Option, } -#[derive(Debug, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct SRIAMUser { #[serde(rename = "accessKey", default)] pub access_key: String, @@ -270,7 +270,7 @@ pub struct SRIAMUser { pub api_version: Option, } -#[derive(Debug, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct SRGroupInfo { #[serde(rename = "updateReq", default)] pub update_req: GroupAddRemove, @@ -346,7 +346,7 @@ pub struct SRCredInfo { pub api_version: Option, } -#[derive(Debug, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct SRIAMItem { #[serde(default)] pub r#type: String, diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index d9604fe6f..fba311ba9 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -2986,6 +2986,18 @@ fn reconcile_site_replication_wiring() -> std::pin::Pin { if state.pending_endpoint_refresh.is_some() || state.pending_remove.is_some() || state.pending_rotation.is_some() @@ -3017,6 +3029,9 @@ fn reconcile_site_replication_wiring() -> std::pin::Pin upsert_site_replication_retry_event(&mut state.retry_queue, &peer, &path, error, None), None => { - dequeue_site_replication_retry_events(&mut state.retry_queue, &peer, &path); + dequeue_site_replication_retry_events_including_escalated(&mut state.retry_queue, &peer, &path); } } Ok(()) @@ -6071,10 +6086,96 @@ fn retry_event_matches(event: &SiteReplicationRetryEvent, peer: &PeerInfo, path: (event.peer_deployment_id == peer.deployment_id || event.peer_endpoint == peer.endpoint) && event.path == path } +const SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH: &str = "internal:retry-snapshot:iam"; +const SITE_REPLICATION_RETRY_BUCKET_METADATA_SNAPSHOT_PATH: &str = "internal:retry-snapshot:bucket-metadata"; + +fn collapsed_retry_queue_path(path: &str) -> Option<&'static str> { + let base_path = path.split_once('?').map(|(base, _)| base).unwrap_or(path); + match base_path { + "/rustfs/admin/v3/site-replication/peer/iam-item" | SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH => { + Some(SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH) + } + "/rustfs/admin/v3/site-replication/peer/bucket-meta" | SITE_REPLICATION_RETRY_BUCKET_METADATA_SNAPSHOT_PATH => { + Some(SITE_REPLICATION_RETRY_BUCKET_METADATA_SNAPSHOT_PATH) + } + _ => None, + } +} + +fn normalize_collapsed_retry_queue_paths(queue: &mut Vec) -> bool { + let mut changed = false; + let mut normalized: Vec = Vec::with_capacity(queue.len()); + for mut event in queue.drain(..) { + if let Some(path) = collapsed_retry_queue_path(&event.path) + && event.path != path + { + event.path = path.to_string(); + changed = true; + } + + let duplicate = normalized.iter().position(|existing| { + existing.path == event.path + && (existing.peer_deployment_id == event.peer_deployment_id || existing.peer_endpoint == event.peer_endpoint) + }); + let Some(index) = duplicate else { + normalized.push(event); + continue; + }; + + changed = true; + let existing = &mut normalized[index]; + let event_is_newer = match (event.updated_at, existing.updated_at) { + (Some(event), Some(existing)) => event >= existing, + (Some(_), None) => true, + _ => false, + }; + if event_is_newer { + let retry_count = existing.retry_count.max(event.retry_count); + *existing = event; + existing.retry_count = retry_count; + } else { + existing.retry_count = existing.retry_count.max(event.retry_count); + } + existing.failed = existing.retry_count >= SITE_REPLICATION_RETRY_FAILED_AFTER; + } + *queue = normalized; + changed +} + +async fn migrate_collapsed_retry_queue_paths() -> S3Result<()> { + update_site_replication_state_when_changed(|state| { + Ok(if normalize_collapsed_retry_queue_paths(&mut state.retry_queue) { + StateCommit::Changed(()) + } else { + StateCommit::Unchanged(()) + }) + }) + .await +} + +#[cfg(test)] fn dequeue_site_replication_retry_events(queue: &mut Vec, peer: &PeerInfo, path: &str) -> usize { settle_site_replication_retry_events(queue, peer, path, None) } +/// Repair-path settlement: also clears snapshot-escalated entries. Running a +/// repair is the operator's explicit accountability transfer for the +/// possibly-unreplayed deletion the marker records; ordinary delivery +/// successes must not clear it (see [`settle_site_replication_retry_events`]). +fn dequeue_site_replication_retry_events_including_escalated( + queue: &mut Vec, + peer: &PeerInfo, + path: &str, +) -> usize { + let before = queue.len(); + let collapsed_path = collapsed_retry_queue_path(path); + queue.retain(|event| { + !retry_event_matches(event, peer, path) + && !collapsed_path.is_some_and(|collapsed_path| retry_event_matches(event, peer, collapsed_path)) + }); + before.saturating_sub(queue.len()) +} + /// Remove the retry events for (peer, path) that `generation` is entitled to /// settle. A successful delivery only proves the peer reached the state the /// delivery carried: while it was in flight another edit can commit, fail its @@ -6090,10 +6191,24 @@ fn settle_site_replication_retry_events( generation: Option, ) -> usize { let before = queue.len(); + let collapsed_path = collapsed_retry_queue_path(path); queue.retain(|event| { if !retry_event_matches(event, peer, path) { return true; } + // A wire-path success identifies no IAM or bucket-metadata entity. + // This also protects legacy rows until the startup migration moves + // them under their internal snapshot path. + if collapsed_path.is_some() { + return true; + } + // A snapshot-escalated entry records a possibly-unreplayed deletion. + // Collapsed paths are shared by every entity, so a later successful + // delivery of a DIFFERENT item proves nothing about the deleted one — + // only a repair settles it (dequeue_..._including_escalated). + if event.last_error == SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER { + return true; + } match (generation, event.edit_generation) { (Some(settled), Some(failed)) => failed > settled, _ => false, @@ -6109,6 +6224,7 @@ fn upsert_site_replication_retry_event( error: &str, generation: Option, ) { + let path = collapsed_retry_queue_path(path).unwrap_or(path); let now = OffsetDateTime::now_utc(); let detail = summarize_peer_error_detail(error); if let Some(event) = queue.iter_mut().find(|event| retry_event_matches(event, peer, path)) { @@ -6171,7 +6287,12 @@ async fn enqueue_site_replication_retry_event_for_generation( let path_owned = path.to_string(); let error_text = error.to_string(); let result = update_site_replication_state(move |state| { - upsert_site_replication_retry_event(&mut state.retry_queue, &peer_owned, &path_owned, &error_text, generation); + // A peer that left the state can never drain its entries again + // (remove_sites already pruned them); recording a late failure for it + // would only pollute retry_stats until the queue cap evicts it. + if state.peers.contains_key(&peer_owned.deployment_id) { + upsert_site_replication_retry_event(&mut state.retry_queue, &peer_owned, &path_owned, &error_text, generation); + } Ok(()) }) .await; @@ -6205,6 +6326,595 @@ fn retry_event_replayed_by_bootstrap(event: &SiteReplicationRetryEvent) -> bool ) } +/// Exponential backoff base for the background retry drain, aligned with the +/// reconcile cadence (`site_replication_reconcile::RECONCILE_INTERVAL`). +const SITE_REPLICATION_RETRY_DRAIN_BASE_BACKOFF_SECS: i64 = 600; +/// Backoff ceiling: a permanently failed peer is still probed daily. +const SITE_REPLICATION_RETRY_DRAIN_MAX_BACKOFF_SECS: i64 = 86_400; + +/// What the background drain may do for one retry event. Everything not +/// representable here is operator territory (manual repair). +#[derive(Debug, Clone, PartialEq, Eq)] +enum RetryDrainAction { + /// Constant-path IAM item deliveries collapse into one queue entry per + /// peer and their bodies are not persisted; the only faithful replay is + /// the current IAM snapshot from the bootstrap plan. + IamSnapshot, + /// Same collapse for bucket-meta deliveries: replay the bucket metadata + /// snapshot from the bootstrap plan. + BucketMetadataSnapshot, + /// A self-contained bucket op the bootstrap plan can re-derive for its + /// bucket (`make-with-versioning` / `configure-replication`). + BucketOpReplay { operation: String, bucket: String }, + /// Re-send the current peer records under a fresh edit generation. + PeerEdit, +} + +#[derive(Clone)] +enum RetrySnapshot { + Iam(Vec), + BucketMetadata(Vec), +} + +impl RetrySnapshot { + fn from_plan(action: &RetryDrainAction, plan: &SiteReplicationBootstrapPlan) -> Option { + match action { + RetryDrainAction::IamSnapshot => Some(Self::Iam(plan.iam_items.clone())), + RetryDrainAction::BucketMetadataSnapshot => Some(Self::BucketMetadata(plan.bucket_items.clone())), + _ => None, + } + } + + fn fingerprint(&self) -> S3Result>> { + let mut payloads = match self { + Self::Iam(items) => items.iter().map(serde_json::to_vec).collect::, _>>(), + Self::BucketMetadata(items) => items.iter().map(serde_json::to_vec).collect::, _>>(), + } + .map_err(|err| S3Error::with_message(S3ErrorCode::InternalError, format!("serialize retry snapshot failed: {err}")))?; + payloads.sort_unstable(); + Ok(payloads) + } + + fn replay_after_change(previous: &Self, fresh: &Self, observed_at: OffsetDateTime) -> Self { + match (previous, fresh) { + (Self::Iam(previous), Self::Iam(fresh)) => { + let fresh_keys: HashSet = fresh.iter().filter_map(iam_snapshot_key).collect(); + let mut replay = fresh.clone(); + for item in previous { + if iam_snapshot_key(item).is_some_and(|key| !fresh_keys.contains(&key)) { + replay.extend(iam_snapshot_tombstones(item, observed_at)); + } + } + Self::Iam(replay) + } + (Self::BucketMetadata(previous), Self::BucketMetadata(fresh)) => { + let fresh_keys: HashSet<(&str, &str)> = fresh + .iter() + .map(|item| (item.bucket.as_str(), item.r#type.as_str())) + .collect(); + let mut replay = fresh.clone(); + for item in previous { + if !fresh_keys.contains(&(item.bucket.as_str(), item.r#type.as_str())) { + replay.push(bucket_metadata_snapshot_tombstone(item, observed_at)); + } + } + Self::BucketMetadata(replay) + } + _ => fresh.clone(), + } + } + + async fn send(&self, transport: &PeerTransport, access_key: &str, secret_key: &str) -> S3Result<()> { + match self { + Self::Iam(items) => { + for item in items { + SiteReplicationRepairTask::Iam(item) + .send(transport, access_key, secret_key) + .await?; + } + } + Self::BucketMetadata(items) => { + for item in items { + SiteReplicationRepairTask::BucketMetadata(item) + .send(transport, access_key, secret_key) + .await?; + } + } + } + Ok(()) + } +} + +#[derive(Hash, PartialEq, Eq)] +enum IamSnapshotKey { + Policy(String), + User(String), + Group(String), + PolicyMapping { target: String, user_type: i64, is_group: bool }, +} + +fn iam_snapshot_key(item: &SRIAMItem) -> Option { + match item.r#type.as_str() { + "policy" => Some(IamSnapshotKey::Policy(item.name.clone())), + "iam-user" => item + .iam_user + .as_ref() + .map(|user| IamSnapshotKey::User(user.access_key.clone())), + "group-info" => item + .group_info + .as_ref() + .map(|group| IamSnapshotKey::Group(group.update_req.group.clone())), + "policy-mapping" => item.policy_mapping.as_ref().map(|mapping| IamSnapshotKey::PolicyMapping { + target: mapping.user_or_group.clone(), + user_type: mapping.user_type, + is_group: mapping.is_group, + }), + _ => None, + } +} + +fn iam_snapshot_tombstones(item: &SRIAMItem, observed_at: OffsetDateTime) -> Vec { + let mut tombstone = item.clone(); + tombstone.updated_at = Some(observed_at); + match item.r#type.as_str() { + "policy" => tombstone.policy = None, + "iam-user" => { + if let Some(user) = tombstone.iam_user.as_mut() { + user.is_delete_req = true; + user.user_req = None; + } + } + "group-info" => { + let Some(group) = tombstone.group_info.as_mut() else { + return Vec::new(); + }; + group.update_req.is_remove = true; + if group.update_req.members.is_empty() { + return vec![tombstone]; + } + let mut delete = tombstone.clone(); + if let Some(group) = delete.group_info.as_mut() { + group.update_req.members.clear(); + } + return vec![tombstone, delete]; + } + "policy-mapping" => { + if let Some(mapping) = tombstone.policy_mapping.as_mut() { + mapping.policy.clear(); + } + } + _ => return Vec::new(), + } + vec![tombstone] +} + +fn bucket_metadata_snapshot_tombstone(item: &SRBucketMeta, observed_at: OffsetDateTime) -> SRBucketMeta { + SRBucketMeta { + r#type: item.r#type.clone(), + bucket: item.bucket.clone(), + updated_at: Some(observed_at), + expiry_updated_at: Some(observed_at), + api_version: item.api_version.clone(), + ..Default::default() + } +} + +const SITE_REPLICATION_RETRY_SNAPSHOT_STABILITY_ATTEMPTS: usize = 3; + +fn classify_site_replication_retry_event(event: &SiteReplicationRetryEvent) -> Option { + let snapshot_action = match event.path.as_str() { + SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH => Some(RetryDrainAction::IamSnapshot), + SITE_REPLICATION_RETRY_BUCKET_METADATA_SNAPSHOT_PATH => Some(RetryDrainAction::BucketMetadataSnapshot), + _ => None, + }; + if snapshot_action.is_some() && event.last_error != SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER { + return snapshot_action; + } + if event.path.starts_with("internal:") { + // Marker records store payloads in `last_error` (legacy + // pending-endpoint-refresh backup and snapshot liabilities); they are + // not drainable delivery failures. + return None; + } + if event.last_error == SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER { + // Already snapshot-replayed once for this failure episode; a possible + // deletion cannot be replayed from a snapshot, so re-sending daily + // proves nothing. A new hook failure overwrites the marker. + return None; + } + let base_path = event.path.split_once('?').map(|(base, _)| base).unwrap_or(&event.path); + match base_path { + "/rustfs/admin/v3/site-replication/peer/iam-item" => Some(RetryDrainAction::IamSnapshot), + "/rustfs/admin/v3/site-replication/peer/bucket-meta" => Some(RetryDrainAction::BucketMetadataSnapshot), + SITE_REPLICATION_PEER_EDIT_PATH => Some(RetryDrainAction::PeerEdit), + SITE_REPLICATION_PEER_BUCKET_OPS_PATH => { + let operation = retry_bucket_operation(&event.path)?; + if !matches!( + operation.as_str(), + SITE_REPLICATION_BUCKET_OP_MAKE_WITH_VERSIONING | SITE_REPLICATION_BUCKET_OP_CONFIGURE_REPLICATION + ) { + // Destructive ops (delete-bucket / force-delete-bucket) are + // operator territory: replaying them against a peer whose + // bucket was since recreated is irreversible. + return None; + } + let bucket = retry_bucket_name(&event.path)?; + Some(RetryDrainAction::BucketOpReplay { operation, bucket }) + } + _ => None, + } +} + +fn retry_bucket_name(path: &str) -> Option { + let (_, query) = path.split_once('?')?; + form_urlencoded::parse(query.as_bytes()) + .find_map(|(key, value)| (key == "bucket" && !value.is_empty()).then(|| value.into_owned())) +} + +/// A collapsed retry event after a stable snapshot resend is escalated with +/// this marker instead of being cleared: the snapshot contains no task for a +/// failed deletion, so remote absence remains operator-visible. Collapsed +/// failures use an internal queue path so ordinary successes and older nodes +/// cannot settle an unrelated entity's liability. +const SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER: &str = "snapshot replayed; a failed deletion cannot be replayed from a snapshot — run site replication repair or re-deliver to settle"; + +/// Escalate a collapsed retry event after its snapshot resend succeeded, +/// unless a newer failure was recorded after `snapshot_updated_at` (that +/// failure belongs to a newer local commit the snapshot did not contain and +/// must keep the entry drain-eligible). +fn escalate_site_replication_retry_events_up_to( + queue: &mut Vec, + peer: &PeerInfo, + path: &str, + snapshot_updated_at: Option, +) -> usize { + let Some(marker_path) = collapsed_retry_queue_path(path) else { + return 0; + }; + + if path != marker_path { + queue.retain(|event| { + if !retry_event_matches(event, peer, path) { + return true; + } + matches!((event.updated_at, snapshot_updated_at), (Some(current), Some(seen)) if current > seen) + || matches!((event.updated_at, snapshot_updated_at), (Some(_), None)) + }); + } + + let marker_index = queue.iter().position(|event| retry_event_matches(event, peer, marker_path)); + let marker_index = marker_index.unwrap_or_else(|| { + queue.push(SiteReplicationRetryEvent { + id: Uuid::new_v4().to_string(), + peer_deployment_id: peer.deployment_id.clone(), + peer_endpoint: peer.endpoint.clone(), + path: marker_path.to_string(), + updated_at: snapshot_updated_at, + ..Default::default() + }); + queue.len() - 1 + }); + let event = &mut queue[marker_index]; + let newer_failure_recorded = match (event.updated_at, snapshot_updated_at) { + (Some(current), Some(seen)) => current > seen, + (Some(_), None) => true, + (None, _) => false, + }; + if newer_failure_recorded && event.last_error != SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER { + return 0; + } + event.failed = true; + event.retry_count = event.retry_count.max(SITE_REPLICATION_RETRY_FAILED_AFTER); + event.last_error = SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER.to_string(); + event.updated_at = Some(OffsetDateTime::now_utc()); + 1 +} + +async fn escalate_site_replication_retry_event_up_to(peer: &PeerInfo, path: &str, snapshot_updated_at: Option) { + let peer_owned = peer.clone(); + let path_owned = path.to_string(); + let result = update_site_replication_state(move |state| { + escalate_site_replication_retry_events_up_to(&mut state.retry_queue, &peer_owned, &path_owned, snapshot_updated_at); + Ok(()) + }) + .await; + + if let Err(err) = result { + warn!( + component = LOG_COMPONENT_ADMIN, + subsystem = LOG_SUBSYSTEM_SITE_REPLICATION, + event = EVENT_ADMIN_SITE_REPLICATION_STATE, + peer = %peer.endpoint, + deployment_id = %peer.deployment_id, + path, + error = ?err, + "failed to escalate site replication retry event" + ); + } +} + +/// Whether the drain may attempt this event now. +fn site_replication_retry_backoff_elapsed(event: &SiteReplicationRetryEvent, now: OffsetDateTime) -> bool { + let Some(updated_at) = event.updated_at else { + return true; + }; + // 600 * 2^8 already exceeds the daily ceiling; capping the shift keeps + // the arithmetic overflow-free for any persisted retry_count. + let exponent = event.retry_count.saturating_sub(1).min(8); + let delay = (SITE_REPLICATION_RETRY_DRAIN_BASE_BACKOFF_SECS << exponent).min(SITE_REPLICATION_RETRY_DRAIN_MAX_BACKOFF_SECS); + now.unix_timestamp().saturating_sub(updated_at.unix_timestamp()) >= delay +} + +/// The subset of the retry queue the background drain is allowed to touch. +fn actionable_site_replication_retry_events(state: &SiteReplicationState, now: OffsetDateTime) -> Vec { + state + .retry_queue + .iter() + .filter(|event| classify_site_replication_retry_event(event).is_some()) + .filter(|event| state.peers.contains_key(&event.peer_deployment_id)) + .filter(|event| site_replication_retry_backoff_elapsed(event, now)) + .cloned() + .collect() +} + +/// Background consumer for the retry queue, run from the reconcile tick. +/// +/// Scope: this settles "delivered once and failed" entries whose replay is +/// faithful (bucket ops, peer edits). Collapsed iam-item / bucket-meta +/// entries are snapshot-resent and then *escalated*, not cleared — a failed +/// deletion leaves no task in the snapshot, so remote absence stays unproven +/// until a later delivery or a manual repair. A hook that never fired (crash +/// between the local commit and the send) leaves no entry at all, so the +/// drain is not a full cross-site diff-heal; manual repair remains the +/// authoritative catch-all. +async fn drain_site_replication_retry_queue() { + if let Err(err) = drain_site_replication_retry_queue_inner().await { + warn!( + event = EVENT_ADMIN_SITE_REPLICATION_STATE, + component = LOG_COMPONENT_ADMIN, + subsystem = LOG_SUBSYSTEM_SITE_REPLICATION, + result = "retry_drain_failed", + error = ?err, + "admin site replication state" + ); + } +} + +async fn drain_site_replication_retry_queue_inner() -> S3Result<()> { + let Some(runtime) = runtime_site_replication_targets().await? else { + return Ok(()); + }; + let actionable = actionable_site_replication_retry_events(&runtime.state, OffsetDateTime::now_utc()); + if actionable.is_empty() { + return Ok(()); + } + let Some(store) = current_object_store_handle() else { + return Ok(()); + }; + if runtime.state.pending_endpoint_refresh.is_some() + || runtime.state.pending_remove.is_some() + || runtime.state.pending_rotation.is_some() + { + // The tick-level gate ran before the reconcilers; a multi-step flow + // (endpoint refresh commits its pending marker without the lifecycle + // guard) may have started since. Re-check on the fresh state. + return Ok(()); + } + // Serialize against operator repair execution. This does NOT close the + // dry-run -> execute window (dry-run takes no lock): a drain settling a + // replayable bucket-op entry in that window changes the preflight token + // and execute fails safe with "preflight is stale" — the operator + // re-runs the dry-run. Lock order matches repair: lifecycle guard (held + // by the reconcile tick) -> repair execution lock -> state object lock + // inside the send bookkeeping. An operator repair holding the lock makes + // this tick skip after the lock-acquire timeout. + with_config_object_write_lock(store, SITE_REPLICATION_REPAIR_EXECUTION_LOCK_PATH.to_string(), move || async move { + drain_site_replication_retry_queue_locked(runtime, actionable).await + }) + .await + .map_err(ApiError::from)? +} + +async fn drain_site_replication_retry_queue_locked( + runtime: SiteReplicationRuntime, + events: Vec, +) -> S3Result<()> { + let needs_plan = events + .iter() + .any(|event| !matches!(classify_site_replication_retry_event(event), Some(RetryDrainAction::PeerEdit))); + // The plan is a full local snapshot (buckets + IAM); build it once per + // tick and only when a snapshot resend is actually due. + let plan = if needs_plan { + let info = build_sr_info(&runtime.state, &runtime.local_peer).await?; + Some(site_replication_bootstrap_plan(&info)?) + } else { + None + }; + + let mut events_by_peer: BTreeMap> = BTreeMap::new(); + for event in events { + events_by_peer + .entry(event.peer_deployment_id.clone()) + .or_default() + .push(event); + } + + let mut settled = 0usize; + let mut failures = 0usize; + for (deployment_id, peer_events) in events_by_peer { + let Some(peer) = runtime.state.peers.get(&deployment_id) else { + continue; + }; + if deployment_id == runtime.local_peer.deployment_id + || same_identity_endpoint(&peer.endpoint, &runtime.local_peer.endpoint) + { + continue; + } + let transport = match PeerTransport::for_runtime_peer(peer).await { + Ok(transport) => transport, + Err(err) => { + // Record the attempt so backoff advances for an unreachable + // peer instead of re-dialing it every tick. + for event in &peer_events { + enqueue_site_replication_retry_event(peer, &event.path, &err).await; + } + failures += peer_events.len(); + continue; + } + }; + for event in peer_events { + let Some(action) = classify_site_replication_retry_event(&event) else { + continue; + }; + match drain_one_site_replication_retry_event(&runtime, peer, &transport, &event, action, plan.as_ref()).await { + Ok(true) => settled += 1, + Ok(false) => {} + Err(_) => failures += 1, + } + } + } + + if settled > 0 || failures > 0 { + info!( + event = EVENT_ADMIN_SITE_REPLICATION_STATE, + component = LOG_COMPONENT_ADMIN, + subsystem = LOG_SUBSYSTEM_SITE_REPLICATION, + result = "retry_drain_settled", + settled, + failures, + "admin site replication state" + ); + } + Ok(()) +} + +/// Replay one retry event against its peer. Returns `Ok(true)` when the +/// event was settled (delivered, or provably stale), `Ok(false)` when it was +/// skipped, and `Err` after a failed delivery (already re-queued with an +/// incremented retry count). +async fn drain_one_site_replication_retry_event( + runtime: &SiteReplicationRuntime, + peer: &PeerInfo, + transport: &PeerTransport, + event: &SiteReplicationRetryEvent, + action: RetryDrainAction, + plan: Option<&SiteReplicationBootstrapPlan>, +) -> S3Result { + let access_key = &runtime.state.service_account_access_key; + let secret_key = &runtime.service_account_secret_key; + match action.clone() { + RetryDrainAction::IamSnapshot | RetryDrainAction::BucketMetadataSnapshot => { + let Some(plan) = plan else { + return Ok(false); + }; + let mut current_snapshot = RetrySnapshot::from_plan(&action, plan).expect("snapshot action has a snapshot"); + let mut replay = current_snapshot.clone(); + for _ in 0..SITE_REPLICATION_RETRY_SNAPSHOT_STABILITY_ATTEMPTS { + let current_fingerprint = current_snapshot.fingerprint()?; + if let Err(err) = replay.send(transport, access_key, secret_key).await { + enqueue_site_replication_retry_event(peer, &event.path, &err).await; + return Err(err); + } + let fresh_info = build_sr_info(&runtime.state, &runtime.local_peer).await?; + let fresh_plan = site_replication_bootstrap_plan(&fresh_info)?; + let fresh_snapshot = RetrySnapshot::from_plan(&action, &fresh_plan).expect("snapshot action has a snapshot"); + if fresh_snapshot.fingerprint()? == current_fingerprint { + escalate_site_replication_retry_event_up_to(peer, &event.path, event.updated_at).await; + return Ok(true); + } + replay = RetrySnapshot::replay_after_change(¤t_snapshot, &fresh_snapshot, OffsetDateTime::now_utc()); + current_snapshot = fresh_snapshot; + } + Ok(false) + } + RetryDrainAction::BucketOpReplay { operation, bucket } => { + let Some(plan) = plan else { + return Ok(false); + }; + // Replay from the CURRENT plan, never the recorded path: the + // recorded query can carry an expired one-shot bootstrap token or + // a stale createdAt. + let make_op = operation == SITE_REPLICATION_BUCKET_OP_MAKE_WITH_VERSIONING; + let paths = if make_op { + &plan.bucket_make_ops + } else { + &plan.bucket_configure_ops + }; + let tasks: Vec> = paths + .iter() + .filter(|path| retry_bucket_name(path).as_deref() == Some(bucket.as_str())) + .map(|path| { + if make_op { + SiteReplicationRepairTask::BucketMake(path) + } else { + SiteReplicationRepairTask::Replication(path) + } + }) + .collect(); + if tasks.is_empty() { + // The bucket left the plan (deleted, or replication no longer + // configured): the recorded intent is stale, settle it. + dequeue_site_replication_retry_event(peer, &event.path).await; + return Ok(true); + } + for task in &tasks { + if let Err(err) = task.send(transport, access_key, secret_key).await { + enqueue_site_replication_retry_event(peer, &event.path, &err).await; + return Err(err); + } + } + dequeue_site_replication_retry_event(peer, &event.path).await; + Ok(true) + } + RetryDrainAction::PeerEdit => { + // The recorded generation is stale by definition — the receiver + // fences it. Allocate a fresh generation and re-send the current + // peer records (a superset of the failed body; the receiver + // upserts), all inside one state transaction so the fence and the + // bodies agree. + let target_id = peer.deployment_id.clone(); + let (generation, bodies) = update_site_replication_state(move |state| { + if !state.peers.contains_key(&target_id) { + return Ok((None, Vec::new())); + } + Ok((Some(next_peer_edit_generation(state)), state.peers.values().cloned().collect::>())) + }) + .await?; + let Some(generation) = generation else { + // Peer left between the snapshot and now; the queue entry was + // already pruned by remove_sites. + return Ok(false); + }; + let local_deployment_id = Some(runtime.local_peer.deployment_id.as_str()).filter(|id| !id.is_empty()); + let edit_path = peer_edit_path_with_fence(local_deployment_id, generation); + let delivery_fence = local_deployment_id.is_some().then_some(generation); + for body in &bodies { + if let Err(err) = send_peer_admin_request_with_client( + &transport.client, + &transport.connection, + &edit_path, + access_key, + secret_key, + body, + ) + .await + { + enqueue_site_replication_retry_event_for_generation( + peer, + SITE_REPLICATION_PEER_EDIT_PATH, + &err, + delivery_fence, + ) + .await; + return Err(err); + } + } + dequeue_site_replication_retry_event_for_generation(peer, SITE_REPLICATION_PEER_EDIT_PATH, delivery_fence).await; + Ok(true) + } + } +} + /// Remove a retry event for (peer, path) from the queue on successful delivery. /// This is a no-op (load + no-op persist skipped) when no matching entry exists, /// avoiding unnecessary I/O on the common path. @@ -11839,6 +12549,320 @@ mod tests { assert!(target_state.peers["remote"].skip_tls_verify); } + fn drain_event(peer: &str, path: &str, retry_count: u32, updated_at: Option) -> SiteReplicationRetryEvent { + SiteReplicationRetryEvent { + id: format!("evt-{peer}"), + peer_deployment_id: peer.to_string(), + peer_endpoint: format!("https://{peer}.example.com"), + path: path.to_string(), + retry_count, + failed: retry_count >= SITE_REPLICATION_RETRY_FAILED_AFTER, + last_error: "remote-operation-failed".to_string(), + updated_at, + edit_generation: None, + } + } + + /// P1-3 red-light: the drain must only ever act on deliveries it can + /// replay faithfully. IAM / bucket-meta entries collapse per (peer, path) + /// with no body persisted — only a snapshot resend is truthful; bucket + /// makes/replication configs are re-derivable; destructive bucket ops and + /// unrelated `internal:` marker records are never background-replayed. + #[test] + fn test_classify_site_replication_retry_event_actions() { + let now = OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp"); + let classify = |path: &str| classify_site_replication_retry_event(&drain_event("remote", path, 1, Some(now))); + + assert_eq!( + classify("/rustfs/admin/v3/site-replication/peer/iam-item"), + Some(RetryDrainAction::IamSnapshot) + ); + assert_eq!( + classify("/rustfs/admin/v3/site-replication/peer/bucket-meta"), + Some(RetryDrainAction::BucketMetadataSnapshot) + ); + assert_eq!(classify(SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH), Some(RetryDrainAction::IamSnapshot)); + assert_eq!( + classify(SITE_REPLICATION_RETRY_BUCKET_METADATA_SNAPSHOT_PATH), + Some(RetryDrainAction::BucketMetadataSnapshot) + ); + assert_eq!(classify(SITE_REPLICATION_PEER_EDIT_PATH), Some(RetryDrainAction::PeerEdit)); + assert_eq!( + classify( + "/rustfs/admin/v3/site-replication/peer/bucket-ops?bucket=photos&operation=make-with-versioning&createdAt=1" + ), + Some(RetryDrainAction::BucketOpReplay { + operation: SITE_REPLICATION_BUCKET_OP_MAKE_WITH_VERSIONING.to_string(), + bucket: "photos".to_string(), + }) + ); + assert_eq!( + classify("/rustfs/admin/v3/site-replication/peer/bucket-ops?bucket=photos&operation=configure-replication"), + Some(RetryDrainAction::BucketOpReplay { + operation: SITE_REPLICATION_BUCKET_OP_CONFIGURE_REPLICATION.to_string(), + bucket: "photos".to_string(), + }) + ); + // Destructive ops are operator territory: replaying a bucket delete + // against a peer whose bucket was since recreated is irreversible. + assert_eq!( + classify("/rustfs/admin/v3/site-replication/peer/bucket-ops?bucket=photos&operation=delete-bucket"), + None + ); + assert_eq!( + classify("/rustfs/admin/v3/site-replication/peer/bucket-ops?bucket=photos&operation=force-delete-bucket"), + None + ); + // `internal:` records store payloads in `last_error`, not failures. + assert_eq!(classify(SITE_REPLICATION_ENDPOINT_REFRESH_RETRY_PATH), None); + assert_eq!(classify("internal:some-future-marker"), None); + assert_eq!(classify("/rustfs/admin/v3/site-replication/peer/unknown"), None); + } + + #[test] + fn test_retry_snapshot_fingerprint_detects_concurrent_iam_change() { + let old = SRIAMItem { + r#type: "policy".to_string(), + name: "readwrite".to_string(), + updated_at: Some(OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp")), + ..Default::default() + }; + let mut new = old.clone(); + new.updated_at = Some(OffsetDateTime::from_unix_timestamp(1_700_000_001).expect("timestamp")); + + let sent = RetrySnapshot::Iam(vec![old]); + let changed = RetrySnapshot::Iam(vec![new]); + assert_ne!(sent.fingerprint().unwrap(), changed.fingerprint().unwrap()); + } + + #[test] + fn test_retry_snapshot_replays_a_concurrent_deletion_as_a_tombstone() { + let observed_at = OffsetDateTime::from_unix_timestamp(1_700_000_010).expect("timestamp"); + let policy = SRIAMItem { + r#type: "policy".to_string(), + name: "readwrite".to_string(), + policy: Some(serde_json::json!({"Version": "2012-10-17"})), + ..Default::default() + }; + let replay = + RetrySnapshot::replay_after_change(&RetrySnapshot::Iam(vec![policy]), &RetrySnapshot::Iam(Vec::new()), observed_at); + let RetrySnapshot::Iam(items) = replay else { + panic!("IAM snapshot expected"); + }; + assert_eq!(items.len(), 1); + assert_eq!(items[0].name, "readwrite"); + assert!(items[0].policy.is_none()); + assert_eq!(items[0].updated_at, Some(observed_at)); + + let bucket = SRBucketMeta { + r#type: "tags".to_string(), + bucket: "photos".to_string(), + tags: Some("encoded-tags".to_string()), + ..Default::default() + }; + let replay = RetrySnapshot::replay_after_change( + &RetrySnapshot::BucketMetadata(vec![bucket]), + &RetrySnapshot::BucketMetadata(Vec::new()), + observed_at, + ); + let RetrySnapshot::BucketMetadata(items) = replay else { + panic!("bucket metadata snapshot expected"); + }; + assert_eq!(items.len(), 1); + assert_eq!(items[0].bucket, "photos"); + assert_eq!(items[0].r#type, "tags"); + assert!(items[0].tags.is_none()); + assert_eq!(items[0].updated_at, Some(observed_at)); + } + + /// Exponential backoff gates every attempt: without it a dead peer's + /// entries hit `failed` (retry_count >= 3) within 30 minutes of reconcile + /// ticks and the retry stats lose their signal. + #[test] + fn test_site_replication_retry_backoff_schedule() { + let now = OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp"); + let at = |secs_ago: i64| Some(now - time::Duration::seconds(secs_ago)); + let elapsed = |retry_count: u32, secs_ago: i64| { + site_replication_retry_backoff_elapsed(&drain_event("remote", "/p", retry_count, at(secs_ago)), now) + }; + + // No record of when it failed: attempt now. + assert!(site_replication_retry_backoff_elapsed(&drain_event("remote", "/p", 1, None), now)); + // First failure: one reconcile interval. + assert!(!elapsed(1, 599)); + assert!(elapsed(1, 601)); + // Third failure: 600 * 2^2 = 2400s. + assert!(!elapsed(3, 1200)); + assert!(elapsed(3, 2401)); + // Ceiling: a long-dead peer is still probed daily, never less often. + assert!(!elapsed(30, 86_000)); + assert!(elapsed(30, 86_401)); + } + + /// The actionable subset respects classification, peer membership and + /// backoff; everything else stays untouched in the queue. + #[test] + fn test_actionable_site_replication_retry_events_filters() { + let now = OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp"); + let old = Some(now - time::Duration::seconds(700)); + let mut state = SiteReplicationState::default(); + state + .peers + .insert("remote".to_string(), peer("remote", "https://remote.example.com")); + + state.retry_queue = vec![ + // Eligible: known peer, replayable, past backoff. + drain_event("remote", SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH, 1, old), + // Not yet due. + drain_event("remote", "/rustfs/admin/v3/site-replication/peer/bucket-meta", 2, Some(now)), + // Unknown peer (removed since the failure was recorded). + drain_event("gone", "/rustfs/admin/v3/site-replication/peer/iam-item", 1, old), + // Marker record, not a delivery failure. + drain_event("remote", SITE_REPLICATION_ENDPOINT_REFRESH_RETRY_PATH, 0, old), + // Destructive op: operator-only. + drain_event( + "remote", + "/rustfs/admin/v3/site-replication/peer/bucket-ops?bucket=photos&operation=delete-bucket", + 1, + old, + ), + ]; + + let actionable = actionable_site_replication_retry_events(&state, now); + assert_eq!(actionable.len(), 1, "only the due, replayable, known-peer event is actionable"); + assert_eq!(actionable[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); + } + + /// The drain settles a peer-edit success under a freshly allocated + /// generation; legacy queue entries carry `edit_generation: None` and + /// must be cleared by that generation-scoped settlement (`(Some, None)` + /// falls through to removal), or the drain would spin on them forever. + #[test] + fn test_settle_clears_legacy_none_generation_event_for_generation_scoped_success() { + let target = peer("remote", "https://remote.example.com"); + let mut queue = vec![drain_event("remote", SITE_REPLICATION_PEER_EDIT_PATH, 1, None)]; + assert!(queue[0].edit_generation.is_none()); + + let settled = settle_site_replication_retry_events(&mut queue, &target, SITE_REPLICATION_PEER_EDIT_PATH, Some(42)); + + assert_eq!(settled, 1, "a legacy None-generation event must settle under a newer generation"); + assert!(queue.is_empty()); + } + + /// A successful snapshot resend cannot prove a failed *deletion* was + /// replayed, so the collapsed entry is escalated (operator-visible, + /// drain-idle) instead of cleared — unless a newer failure was stamped + /// during the delivery window, which keeps the entry drain-eligible. + #[test] + fn test_escalate_up_to_marks_snapshot_replayed_and_keeps_newer_failures() { + let target = peer("remote", "https://remote.example.com"); + let path = "/rustfs/admin/v3/site-replication/peer/iam-item"; + let snapshot_at = OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp"); + + // Failure re-stamped after the snapshot: untouched, still eligible. + let mut queue = vec![drain_event( + "remote", + SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH, + 2, + Some(snapshot_at + time::Duration::seconds(5)), + )]; + assert_eq!( + escalate_site_replication_retry_events_up_to( + &mut queue, + &target, + SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH, + Some(snapshot_at), + ), + 0 + ); + assert!(!queue[0].failed); + assert!( + classify_site_replication_retry_event(&queue[0]).is_some(), + "a newer failure must stay drain-eligible" + ); + + // Unchanged since the snapshot: escalated, kept, drain-idle. + let mut queue = vec![drain_event( + "remote", + SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH, + 2, + Some(snapshot_at), + )]; + assert_eq!( + escalate_site_replication_retry_events_up_to(&mut queue, &target, path, Some(snapshot_at)), + 1 + ); + assert_eq!(queue.len(), 1, "the entry must survive until remote absence is proven"); + assert_eq!(queue[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); + assert!(queue[0].failed); + assert_eq!(queue[0].last_error, SITE_REPLICATION_RETRY_SNAPSHOT_REPLAYED_MARKER); + assert!( + classify_site_replication_retry_event(&queue[0]).is_none(), + "a snapshot-replayed entry must not be re-sent daily" + ); + // Ordinary success dequeues must not clear the marker: collapsed + // paths are shared by every entity, so a successful Bob update + // proves nothing about a failed Alice deletion (second review + // round). + assert_eq!(dequeue_site_replication_retry_events(&mut queue, &target, path), 0); + assert_eq!(queue.len(), 1, "an escalated entry must survive an ordinary delivery success"); + // Only a repair — the operator's accountability transfer — settles it. + assert_eq!(dequeue_site_replication_retry_events_including_escalated(&mut queue, &target, path), 1); + assert!(queue.is_empty()); + + // A failed Alice deletion is stored under the internal path, so a + // successful Bob update on the shared wire path cannot erase it even + // before the drain runs. + let mut queue = Vec::new(); + upsert_site_replication_retry_event(&mut queue, &target, path, "alice delete failed", None); + assert_eq!(queue[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); + assert_eq!(dequeue_site_replication_retry_events(&mut queue, &target, path), 0); + assert_eq!(queue.len(), 1); + assert_eq!(queue[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); + + // A later hook failure overwrites the marker and re-arms the drain. + let mut queue = vec![drain_event("remote", path, 2, Some(snapshot_at))]; + escalate_site_replication_retry_events_up_to(&mut queue, &target, path, Some(snapshot_at)); + upsert_site_replication_retry_event(&mut queue, &target, path, "peer offline", None); + assert!(classify_site_replication_retry_event(&queue[0]).is_some()); + + // Legacy entry without a timestamp: escalated. + let mut queue = vec![drain_event("remote", path, 2, None)]; + assert_eq!( + escalate_site_replication_retry_events_up_to(&mut queue, &target, path, Some(snapshot_at)), + 1 + ); + + // A cloned event can disappear during replay; escalation recreates + // the internal liability while leaving another peer's row untouched. + let mut queue = vec![drain_event("other", path, 2, Some(snapshot_at))]; + assert_eq!( + escalate_site_replication_retry_events_up_to(&mut queue, &target, path, Some(snapshot_at)), + 1 + ); + assert!(!queue[0].failed); + assert_eq!(queue.len(), 2); + assert_eq!(queue[1].peer_deployment_id, target.deployment_id); + assert_eq!(queue[1].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); + } + + #[test] + fn test_collapsed_retry_queue_migration_preserves_legacy_liability() { + let peer = PeerInfo { + deployment_id: "remote-dep".to_string(), + ..peer("remote", "https://remote.example.com") + }; + let wire_path = "/rustfs/admin/v3/site-replication/peer/iam-item"; + let now = OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp"); + let mut queue = vec![drain_event("remote-dep", wire_path, 2, Some(now))]; + + assert_eq!(dequeue_site_replication_retry_events(&mut queue, &peer, wire_path), 0); + assert!(normalize_collapsed_retry_queue_paths(&mut queue)); + assert_eq!(queue.len(), 1); + assert_eq!(queue[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); + assert!(!normalize_collapsed_retry_queue_paths(&mut queue)); + } + #[test] fn test_pending_endpoint_refresh_retry_summary_redacts_pem() { let pem = "-----BEGIN CERTIFICATE-----\nsecret-marker\n-----END CERTIFICATE-----"; @@ -13545,6 +14569,7 @@ mod tests { upsert_site_replication_retry_event(&mut queue, &peer, "/rustfs/admin/v3/site-replication/peer/iam-item", "third", None); assert_eq!(queue.len(), 1); + assert_eq!(queue[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); assert_eq!(queue[0].retry_count, SITE_REPLICATION_RETRY_FAILED_AFTER); assert!(queue[0].failed); assert_eq!(queue[0].last_error, "third"); @@ -13586,12 +14611,12 @@ mod tests { ); assert!(queue.is_empty()); - // Broadcast paths carry no generation and keep settling unconditionally - // — their retry events live under their own path and never collide - // with a peer-edit delivery. + // Collapsed broadcast failures live under an internal snapshot path; + // an unrelated success on their shared wire path cannot settle them. let iam_path = "/rustfs/admin/v3/site-replication/peer/iam-item"; upsert_site_replication_retry_event(&mut queue, &peer, iam_path, "peer offline", None); - assert_eq!(dequeue_site_replication_retry_events(&mut queue, &peer, iam_path), 1); + assert_eq!(dequeue_site_replication_retry_events(&mut queue, &peer, iam_path), 0); + assert_eq!(queue[0].path, SITE_REPLICATION_RETRY_IAM_SNAPSHOT_PATH); } /// P1-15 review follow-up: the receiving side of the ordering fence. Two @@ -13910,7 +14935,7 @@ mod tests { deployment_id: "current-dep".to_string(), ..peer("remote", "https://remote.example.com") }; - let path = "/rustfs/admin/v3/site-replication/peer/iam-item"; + let path = SITE_REPLICATION_PEER_EDIT_PATH; let mut queue = vec![ SiteReplicationRetryEvent { id: "same-endpoint".to_string(), @@ -17167,17 +18192,31 @@ mod tests { async fn test_retry_event_persist_must_not_wipe_concurrent_locked_rmw() { publish_ready_iam_context().await; + const ROUNDS: usize = 8; let seed = SiteReplicationState { pending_rotation: Some(PendingRotation { id: "rot-1".to_string(), access_key: "svc-account".to_string(), ..Default::default() }), + // Retry events are only recorded for current peers; seed them so + // the concurrency assertion below exercises the persist path. + peers: (0..ROUNDS) + .map(|round| { + let deployment_id = format!("peer-{round}-deployment"); + ( + deployment_id.clone(), + PeerInfo { + endpoint: format!("https://peer-{round}.example:9000"), + deployment_id, + ..Default::default() + }, + ) + }) + .collect(), ..Default::default() }; save_site_replication_state(&seed).await.expect("seed state"); - - const ROUNDS: usize = 8; for round in 0..ROUNDS { let peer = PeerInfo { endpoint: format!("https://peer-{round}.example:9000"), From d091554ffe73d7d82b29f4c32433f101cd7130fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Mon, 17 Aug 2026 14:12:39 +0800 Subject: [PATCH 05/10] fix(kms): resolve Vault auth from the environment at startup and add Kubernetes auth (#6095) --- crates/kms/src/api_types.rs | 58 +++ crates/kms/src/backends/vault.rs | 1 + crates/kms/src/backends/vault_credentials.rs | 276 ++++++++++++++- crates/kms/src/backends/vault_transit.rs | 1 + crates/kms/src/backup/vault_restore.rs | 4 + crates/kms/src/config.rs | 349 ++++++++++++++++--- docs/operations/kms-backend-security.md | 2 +- docs/operations/vault-kms-authentication.md | 54 ++- rustfs/src/admin/handlers/kms_backup.rs | 6 +- rustfs/src/init.rs | 215 ++++++++++-- 10 files changed, 862 insertions(+), 104 deletions(-) diff --git a/crates/kms/src/api_types.rs b/crates/kms/src/api_types.rs index b53aa4f34..6faf6cacc 100644 --- a/crates/kms/src/api_types.rs +++ b/crates/kms/src/api_types.rs @@ -293,6 +293,15 @@ enum StrictVaultAuthMethod { #[serde(default)] refresh_safety_window_secs: Option, }, + Kubernetes { + role: String, + #[serde(default)] + mount: Option, + #[serde(default)] + jwt_path: Option, + #[serde(default)] + refresh_safety_window_secs: Option, + }, TokenFile { path: std::path::PathBuf, #[serde(default)] @@ -319,6 +328,17 @@ impl From for VaultAuthMethod { mount: mount.unwrap_or_else(|| crate::config::DEFAULT_VAULT_APPROLE_MOUNT.to_string()), refresh_safety_window_secs, }, + StrictVaultAuthMethod::Kubernetes { + role, + mount, + jwt_path, + refresh_safety_window_secs, + } => Self::Kubernetes { + role, + mount: mount.unwrap_or_else(|| crate::config::DEFAULT_VAULT_KUBERNETES_MOUNT.to_string()), + jwt_path: jwt_path.unwrap_or_else(|| std::path::PathBuf::from(crate::config::DEFAULT_VAULT_KUBERNETES_JWT_PATH)), + refresh_safety_window_secs, + }, StrictVaultAuthMethod::TokenFile { path, poll_interval_secs, @@ -499,6 +519,7 @@ impl From<&KmsConfig> for KmsConfigSummary { auth_method_type: match &vault_config.auth_method { VaultAuthMethod::Token { .. } => "token".to_string(), VaultAuthMethod::AppRole { .. } => "approle".to_string(), + VaultAuthMethod::Kubernetes { .. } => "kubernetes".to_string(), VaultAuthMethod::TokenFile { .. } => "token_file".to_string(), }, has_stored_credentials: true, @@ -513,6 +534,7 @@ impl From<&KmsConfig> for KmsConfigSummary { auth_method_type: match &vault_config.auth_method { VaultAuthMethod::Token { .. } => "token".to_string(), VaultAuthMethod::AppRole { .. } => "approle".to_string(), + VaultAuthMethod::Kubernetes { .. } => "kubernetes".to_string(), VaultAuthMethod::TokenFile { .. } => "token_file".to_string(), }, has_stored_credentials: true, @@ -901,6 +923,42 @@ mod tests { assert!(request.to_kms_config().validate().is_ok()); } + /// The admin API reaches Kubernetes auth with the role alone; the mount and + /// the projected token path fall back to the cluster defaults, so a Tenant + /// manifest carries no credential and no cluster-specific paths. + #[test] + fn test_deserialize_vault_configure_request_accepts_kubernetes_auth() { + let raw = serde_json::json!({ + "backend_type": "vault-transit", + "address": "https://vault.example.com:8200", + "mount_path": "rustfs", + "auth_method": { "Kubernetes": { "role": "rustfs" } } + }); + + let request: ConfigureKmsRequest = serde_json::from_value(raw).expect("kubernetes auth should deserialize"); + let config = request.to_kms_config(); + config.validate().expect("kubernetes auth must validate"); + + let vault = config.vault_transit_config().expect("vault transit backend config"); + let VaultAuthMethod::Kubernetes { + role, mount, jwt_path, .. + } = &vault.auth_method + else { + panic!("expected Kubernetes auth, got {:?}", vault.auth_method); + }; + assert_eq!(role, "rustfs"); + assert_eq!(mount, crate::config::DEFAULT_VAULT_KUBERNETES_MOUNT); + assert_eq!(jwt_path, std::path::Path::new(crate::config::DEFAULT_VAULT_KUBERNETES_JWT_PATH)); + + let unknown_field = serde_json::json!({ + "backend_type": "vault-transit", + "address": "https://vault.example.com:8200", + "auth_method": { "Kubernetes": { "role": "rustfs", "service_account": "rustfs" } } + }); + serde_json::from_value::(unknown_field) + .expect_err("an unknown auth field must be rejected rather than silently dropped"); + } + #[test] fn test_deserialize_aws_configure_request_accepts_type_aliases() { for backend_type in ["AWS", "AwsKms", "aws", "aws-kms", "aws_kms"] { diff --git a/crates/kms/src/backends/vault.rs b/crates/kms/src/backends/vault.rs index 84ba8e6fa..56d2a856e 100644 --- a/crates/kms/src/backends/vault.rs +++ b/crates/kms/src/backends/vault.rs @@ -550,6 +550,7 @@ impl VaultKmsClient { address: config.address.clone(), namespace: config.namespace.clone(), attempt_timeout: kms_config.effective_timeout(), + skip_tls_verify: config.tls.as_ref().is_some_and(|tls| tls.skip_verify), }; let source = token_source_for(&config.auth_method, &settings)?; let policy = VaultCredentialPolicy::from_kms_config( diff --git a/crates/kms/src/backends/vault_credentials.rs b/crates/kms/src/backends/vault_credentials.rs index b8766442b..d5d8a564f 100644 --- a/crates/kms/src/backends/vault_credentials.rs +++ b/crates/kms/src/backends/vault_credentials.rs @@ -326,6 +326,97 @@ impl fmt::Debug for AppRoleLogin { } } +/// Token source for [`VaultAuthMethod::Kubernetes`]: exchanges the pod's +/// projected ServiceAccount token for a lease-bound Vault token. +/// +/// The JWT is re-read on every login because the kubelet rotates a projected +/// token well inside the pod's lifetime; caching it would strand the source on +/// an expired assertion once the current Vault token can no longer be renewed. +/// +/// Unlike [`TokenFileSource`], the file mode is not checked: the kubelet owns +/// the projected token and mounts it world-readable by default, so rejecting +/// group/other bits would refuse every standard pod rather than catch a +/// deployment error. +pub(crate) struct KubernetesLogin { + /// Unauthenticated client used only for the login exchange. + login_client: VaultClient, + mount: String, + role: String, + jwt_path: PathBuf, +} + +impl KubernetesLogin { + pub(crate) fn new(settings: &VaultConnectionSettings, mount: String, role: String, jwt_path: PathBuf) -> Result { + Ok(Self { + login_client: settings.build_login_client()?, + mount, + role, + jwt_path, + }) + } + + /// Read the ServiceAccount token for one login attempt. + /// + /// Mirrors [`AppRoleLogin::resolve_secret_id`]: a read failure is fatal for + /// the attempt but the refresh loop keeps retrying, so a token the kubelet + /// has not projected yet heals the source without a restart. + async fn resolve_jwt(&self) -> AttemptResult { + let mut raw = tokio::fs::read_to_string(&self.jwt_path) + .await + .map_err(|error| AttemptError { + class: ErrorClass::Fatal, + error: KmsError::configuration_error(format!( + "Failed to read Kubernetes ServiceAccount token {}: {error}", + self.jwt_path.display() + )), + })?; + let trimmed = raw.trim(); + if trimmed.is_empty() { + raw.zeroize(); + return Err(AttemptError { + class: ErrorClass::Fatal, + error: KmsError::configuration_error(format!( + "Kubernetes ServiceAccount token {} is empty", + self.jwt_path.display() + )), + }); + } + let jwt = SecretString::new(trimmed.to_string()); + raw.zeroize(); + Ok(jwt) + } +} + +#[async_trait] +impl TokenSource for KubernetesLogin { + async fn acquire(&self) -> AttemptResult { + let jwt = self.resolve_jwt().await?; + let auth = vaultrs::auth::kubernetes::login(&self.login_client, &self.mount, &self.role, jwt.expose()) + .await + .map_err(|error| attempt_error("Kubernetes login", error))?; + Ok(TokenLease::from_auth(auth)) + } + + async fn renew(&self, client: &VaultClient) -> AttemptResult { + let auth = vaultrs::token::renew_self(client, None) + .await + .map_err(|error| attempt_error("token renewal", error))?; + Ok(TokenLease::from_auth(auth)) + } +} + +impl fmt::Debug for KubernetesLogin { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // The login client embeds Vault client settings and must stay out of + // Debug output; the role name is not a secret, and the JWT is never held. + f.debug_struct("KubernetesLogin") + .field("mount", &self.mount) + .field("role", &self.role) + .field("jwt_path", &self.jwt_path) + .finish_non_exhaustive() + } +} + /// Token source for [`VaultAuthMethod::TokenFile`]: reads an agent-managed /// token file (for example a Vault Agent auto-auth sink). /// @@ -464,6 +555,9 @@ pub(crate) fn token_source_for( secret_id.clone(), secret_id_file.clone(), )?)), + VaultAuthMethod::Kubernetes { + role, mount, jwt_path, .. + } => Ok(Box::new(KubernetesLogin::new(settings, mount.clone(), role.clone(), jwt_path.clone())?)), VaultAuthMethod::TokenFile { path, poll_interval_secs, @@ -486,6 +580,9 @@ pub(crate) struct VaultConnectionSettings { pub(crate) namespace: Option, /// Per-attempt HTTP timeout applied to the underlying reqwest client. pub(crate) attempt_timeout: Duration, + /// Whether to accept an unverified Vault server certificate. Gated on + /// `allow_insecure_dev_defaults` by `KmsConfig::validate`. + pub(crate) skip_tls_verify: bool, } impl VaultConnectionSettings { @@ -499,6 +596,11 @@ impl VaultConnectionSettings { // operation-level retry policy. settings_builder.timeout(Some(self.attempt_timeout)); settings_builder.token(token); + // Always set explicitly: left unset, vaultrs derives this from its own + // VAULT_SKIP_VERIFY variable, so a stray value in the environment would + // disable certificate verification behind the KMS configuration and its + // insecure-defaults gate. + settings_builder.verify(!self.skip_tls_verify); if let Some(namespace) = &self.namespace { settings_builder.namespace(Some(namespace.clone())); @@ -551,6 +653,10 @@ impl VaultCredentialPolicy { refresh_safety_window_secs: Some(secs), .. } + | VaultAuthMethod::Kubernetes { + refresh_safety_window_secs: Some(secs), + .. + } | VaultAuthMethod::TokenFile { refresh_safety_window_secs: Some(secs), .. @@ -584,15 +690,25 @@ pub(crate) struct VaultClientHandle { impl VaultClientHandle { /// Absolute expiry of this generation's token. + /// + /// `lease.ttl` is built from the `lease_duration` the Vault server sent, so + /// a value too large to add to `issued_at` would panic on the bare `+`. A + /// TTL that cannot be represented is indistinguishable from no expiry, so it + /// collapses to `None` — the same answer already given for the zero-lease + /// tokens Vault issues, which keeps the token in use and still fully + /// validated by Vault on every call. fn expires_at(&self) -> Option { - self.lease.map(|lease| self.issued_at + lease.ttl) + self.lease.and_then(|lease| self.issued_at.checked_add(lease.ttl)) } /// When the renewal task should refresh this generation: half the TTL, /// leaving the second half as budget for retries before the fail-closed /// window is reached. + /// + /// Unrepresentable TTLs collapse to `None` as in [`Self::expires_at`], + /// leaving a token that never expires with nothing to renew. fn renew_at(&self) -> Option { - self.lease.map(|lease| self.issued_at + lease.ttl / 2) + self.lease.and_then(|lease| self.issued_at.checked_add(lease.ttl / 2)) } } @@ -662,7 +778,7 @@ impl VaultCredentialProvider { let handle = self.current.load_full(); if let Some(expires_at) = handle.expires_at() { let now = Instant::now(); - if now + self.policy.safety_window >= expires_at { + if self.inside_safety_window(now, expires_at) { return Err(KmsError::credentials_unavailable(format!( "Vault token (generation {}) is within {:?} of expiry and has not been refreshed; refusing to use it", handle.generation, self.policy.safety_window @@ -672,6 +788,18 @@ impl VaultCredentialProvider { Ok(handle) } + /// Whether the token expiring at `expires_at` is close enough to refuse. + /// + /// `safety_window` reaches here from persisted configuration, so it is not + /// guaranteed to have passed this version's validation: a window too large + /// to add to the current instant would panic on the bare `+`. Such a window + /// means every token is always inside it, so saturating to "refuse" is both + /// the fail-closed answer and the one the arithmetic was reaching for. + fn inside_safety_window(&self, now: Instant, expires_at: Instant) -> bool { + now.checked_add(self.policy.safety_window) + .is_none_or(|deadline| deadline >= expires_at) + } + /// Publish the credential gauges for the generation currently installed. /// /// The fail-closed gauge re-evaluates the very gate @@ -683,7 +811,7 @@ impl VaultCredentialProvider { let fail_closed = match handle.expires_at() { Some(expires_at) => { metrics::gauge!(METRIC_TOKEN_TTL_SECONDS).set(expires_at.saturating_duration_since(now).as_secs_f64()); - now + self.policy.safety_window >= expires_at + self.inside_safety_window(now, expires_at) } // A generation without an expiry has no remaining TTL to report // and can never lapse, so it can never fail closed either. @@ -860,7 +988,7 @@ impl Drop for CredentialTaskHandle { #[cfg(test)] mod tests { use super::*; - use crate::config::REDACTED_SECRET; + use crate::config::{DEFAULT_VAULT_KUBERNETES_MOUNT, REDACTED_SECRET}; use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; const TEST_TOKEN: &str = "vault-token-debug-leak-canary"; @@ -871,6 +999,7 @@ mod tests { address: "http://127.0.0.1:8200".to_string(), namespace: Some("team-namespace".to_string()), attempt_timeout: Duration::from_secs(30), + skip_tls_verify: false, } } @@ -1057,6 +1186,143 @@ mod tests { assert!(format!("{source:?}").contains("AppRoleLogin")); } + #[tokio::test] + async fn test_kubernetes_auth_method_maps_to_login_source() { + let settings = test_settings(); + let source = token_source_for(&VaultAuthMethod::kubernetes("rustfs".to_string()), &settings) + .expect("kubernetes auth must map to a login source"); + + assert!(format!("{source:?}").contains("KubernetesLogin")); + } + + /// `refresh_safety_window_secs` is operator-supplied and reaches the request + /// path from persisted configuration, so the fail-closed comparison must + /// survive a window too large to add to the current instant. Before the + /// checked arithmetic this panicked with "overflow when adding duration to + /// instant" on the first request after a lease-bearing login. + #[tokio::test] + async fn test_current_refuses_rather_than_panics_on_an_unrepresentable_safety_window() { + let (provider, _state) = scripted_provider( + Duration::from_secs(60), + true, + test_policy(Duration::from_secs(u64::MAX), Duration::from_secs(5)), + ) + .await; + + let error = provider + .current() + .expect_err("a window wider than any lease must refuse the token"); + assert!( + matches!(error, KmsError::CredentialsUnavailable { .. }), + "expected CredentialsUnavailable, got {error:?}" + ); + } + + /// `lease_duration` is a bare u64 straight off the Vault response and forms + /// the other side of the same comparison, so an absurd one must not panic + /// either. It is indistinguishable from a non-expiring token, which is how + /// the zero-lease case already behaves. + #[tokio::test] + async fn test_an_unrepresentable_lease_is_treated_as_non_expiring() { + let (provider, _state) = scripted_provider( + Duration::from_secs(u64::MAX), + true, + test_policy(Duration::from_secs(30), Duration::from_secs(5)), + ) + .await; + + provider + .current() + .expect("a token whose expiry cannot be represented must stay usable"); + } + + /// The configured flag has to reach the HTTP client, not just the config + /// struct: every generation (authenticated and login) builds its own client, + /// and a Vault with a self-signed certificate fails the handshake unless + /// each one carries the setting. + #[test] + fn test_skip_tls_verify_reaches_every_vault_client_generation() { + for skip_tls_verify in [false, true] { + let settings = VaultConnectionSettings { + address: "https://vault.example.com:8200".to_string(), + namespace: None, + attempt_timeout: Duration::from_secs(30), + skip_tls_verify, + }; + + let authenticated = settings.build_client(TEST_TOKEN).expect("authenticated client must build"); + assert_eq!(authenticated.settings.verify, !skip_tls_verify); + + let login = settings.build_login_client().expect("login client must build"); + assert_eq!(login.settings.verify, !skip_tls_verify); + } + } + + /// vaultrs derives `verify` from its own VAULT_SKIP_VERIFY variable when the + /// builder leaves it unset, which would disable certificate verification + /// without passing the KMS insecure-defaults gate. + #[test] + fn test_vaultrs_skip_verify_env_cannot_override_the_configured_setting() { + temp_env::with_var("VAULT_SKIP_VERIFY", Some("true"), || { + let client = test_settings().build_client(TEST_TOKEN).expect("client must build"); + assert!( + client.settings.verify, + "a stray VAULT_SKIP_VERIFY must not disable verification behind the KMS configuration" + ); + }); + } + + /// The projected token is read fresh per login attempt and trimmed, so a + /// kubelet rotation is picked up without a restart and a trailing newline + /// does not corrupt the assertion sent to Vault. + #[tokio::test] + async fn test_kubernetes_login_rereads_and_trims_the_service_account_token() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("token"); + tokio::fs::write(&path, " first-jwt\n").await.expect("write token"); + + let login = KubernetesLogin::new( + &test_settings(), + DEFAULT_VAULT_KUBERNETES_MOUNT.to_string(), + "rustfs".to_string(), + path.clone(), + ) + .expect("login source must build"); + + assert_eq!(login.resolve_jwt().await.expect("first read").expose(), "first-jwt"); + + tokio::fs::write(&path, "rotated-jwt").await.expect("rotate token"); + assert_eq!( + login.resolve_jwt().await.expect("second read").expose(), + "rotated-jwt", + "a rotated projected token must be picked up without a restart" + ); + } + + /// The ServiceAccount token is re-read per attempt, so an unreadable or + /// empty one fails that attempt without reaching Vault; the refresh loop + /// keeps retrying, which is what lets a late projection heal the source. + #[tokio::test] + async fn test_kubernetes_login_rejects_an_unusable_service_account_token() { + let dir = tempfile::tempdir().expect("temp dir"); + let missing = dir.path().join("absent-token"); + let empty = dir.path().join("empty-token"); + tokio::fs::write(&empty, " \n").await.expect("write empty token"); + + for (path, expected) in [(missing, "Failed to read"), (empty, "is empty")] { + let login = + KubernetesLogin::new(&test_settings(), DEFAULT_VAULT_KUBERNETES_MOUNT.to_string(), "rustfs".to_string(), path) + .expect("login source must build"); + + let error = login + .acquire() + .await + .expect_err("an unusable ServiceAccount token must fail the attempt"); + assert!(matches!(error.class, ErrorClass::Fatal)); + assert!(error.error.to_string().contains(expected), "got {}", error.error); + } + } + #[tokio::test(start_paused = true)] async fn test_renewal_task_renews_at_half_ttl() { let (provider, state) = scripted_provider( diff --git a/crates/kms/src/backends/vault_transit.rs b/crates/kms/src/backends/vault_transit.rs index 095fcf455..602c53154 100644 --- a/crates/kms/src/backends/vault_transit.rs +++ b/crates/kms/src/backends/vault_transit.rs @@ -415,6 +415,7 @@ impl VaultTransitKmsClient { address: config.address.clone(), namespace: config.namespace.clone(), attempt_timeout: kms_config.effective_timeout(), + skip_tls_verify: config.tls.as_ref().is_some_and(|tls| tls.skip_verify), }; let source = token_source_for(&config.auth_method, &settings)?; let policy = VaultCredentialPolicy::from_kms_config( diff --git a/crates/kms/src/backup/vault_restore.rs b/crates/kms/src/backup/vault_restore.rs index b7d4884cc..b033c4d5f 100644 --- a/crates/kms/src/backup/vault_restore.rs +++ b/crates/kms/src/backup/vault_restore.rs @@ -450,6 +450,10 @@ impl VaultRestoreClient { address: target.address.clone(), namespace: target.namespace.clone(), attempt_timeout: kms_config.effective_timeout(), + // A restore target carries no TLS settings, so certificates are + // always verified: recovery is the last path that should accept an + // unauthenticated Vault. + skip_tls_verify: false, }; let source = token_source_for(&target.auth_method, &settings)?; let policy = VaultCredentialPolicy::from_kms_config( diff --git a/crates/kms/src/config.rs b/crates/kms/src/config.rs index c33d1ae0a..bd6c4a0e1 100644 --- a/crates/kms/src/config.rs +++ b/crates/kms/src/config.rs @@ -25,6 +25,10 @@ use url::Url; pub const ENV_KMS_ALLOW_INSECURE_DEV_DEFAULTS: &str = "RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS"; pub const ENV_KMS_ALLOW_IMMEDIATE_DELETION: &str = "RUSTFS_KMS_ALLOW_IMMEDIATE_DELETION"; +pub const ENV_KMS_VAULT_ADDRESS: &str = "RUSTFS_KMS_VAULT_ADDRESS"; +pub const ENV_KMS_VAULT_TOKEN: &str = "RUSTFS_KMS_VAULT_TOKEN"; +pub const ENV_KMS_VAULT_NAMESPACE: &str = "RUSTFS_KMS_VAULT_NAMESPACE"; +pub const ENV_KMS_VAULT_MOUNT_PATH: &str = "RUSTFS_KMS_VAULT_MOUNT_PATH"; pub const ENV_KMS_VAULT_SKIP_TLS_VERIFY: &str = "RUSTFS_KMS_VAULT_SKIP_TLS_VERIFY"; pub const ENV_KMS_VAULT_TRANSIT_METADATA_KV_MOUNT: &str = "RUSTFS_KMS_VAULT_TRANSIT_METADATA_KV_MOUNT"; pub const ENV_KMS_VAULT_TRANSIT_METADATA_PREFIX: &str = "RUSTFS_KMS_VAULT_TRANSIT_METADATA_PREFIX"; @@ -35,6 +39,9 @@ pub const ENV_KMS_VAULT_APPROLE_SECRET_ID: &str = "RUSTFS_KMS_VAULT_APPROLE_SECR pub const ENV_KMS_VAULT_APPROLE_SECRET_ID_FILE: &str = "RUSTFS_KMS_VAULT_APPROLE_SECRET_ID_FILE"; pub const ENV_KMS_VAULT_APPROLE_MOUNT: &str = "RUSTFS_KMS_VAULT_APPROLE_MOUNT"; pub const ENV_KMS_VAULT_TOKEN_FILE: &str = "RUSTFS_KMS_VAULT_TOKEN_FILE"; +pub const ENV_KMS_VAULT_KUBERNETES_ROLE: &str = "RUSTFS_KMS_VAULT_KUBERNETES_ROLE"; +pub const ENV_KMS_VAULT_KUBERNETES_MOUNT: &str = "RUSTFS_KMS_VAULT_KUBERNETES_MOUNT"; +pub const ENV_KMS_VAULT_KUBERNETES_JWT_PATH: &str = "RUSTFS_KMS_VAULT_KUBERNETES_JWT_PATH"; pub const ENV_KMS_AWS_REGION: &str = "RUSTFS_KMS_AWS_REGION"; pub const ENV_KMS_AWS_ENDPOINT_URL: &str = "RUSTFS_KMS_AWS_ENDPOINT_URL"; /// Age in whole seconds beyond which a key is reported as due for rotation; @@ -45,6 +52,9 @@ pub const ENV_KMS_ROTATION_MAX_WRAPS: &str = "RUSTFS_KMS_ROTATION_MAX_WRAPS"; pub const DEFAULT_VAULT_TRANSIT_METADATA_KV_MOUNT: &str = "secret"; pub const DEFAULT_VAULT_TRANSIT_METADATA_KEY_PREFIX: &str = "rustfs/kms/transit-metadata"; pub const DEFAULT_VAULT_APPROLE_MOUNT: &str = "approle"; +pub const DEFAULT_VAULT_KUBERNETES_MOUNT: &str = "kubernetes"; +/// Where the kubelet projects a pod's ServiceAccount token by default. +pub const DEFAULT_VAULT_KUBERNETES_JWT_PATH: &str = "/var/run/secrets/kubernetes.io/serviceaccount/token"; /// Upper bound applied to `KmsConfig::timeout` when deriving backend behavior. /// @@ -84,6 +94,14 @@ fn default_vault_approle_mount() -> String { DEFAULT_VAULT_APPROLE_MOUNT.to_string() } +fn default_vault_kubernetes_mount() -> String { + DEFAULT_VAULT_KUBERNETES_MOUNT.to_string() +} + +fn default_vault_kubernetes_jwt_path() -> PathBuf { + PathBuf::from(DEFAULT_VAULT_KUBERNETES_JWT_PATH) +} + pub const KMS_CONFIG_REDACTION_RULES: &[RedactionRule] = &[ RedactionRule::new("kms.local.master_key", RedactionLevel::Secret, "local backend key encryption material"), RedactionRule::new("kms.vault.token", RedactionLevel::Secret, "vault authentication token"), @@ -490,6 +508,23 @@ pub enum VaultAuthMethod { #[serde(default)] refresh_safety_window_secs: Option, }, + /// Kubernetes authentication: the pod's ServiceAccount token is exchanged + /// for a lease-bound Vault token that is renewed in the background. + Kubernetes { + /// Vault role bound to this ServiceAccount. + role: String, + /// Kubernetes auth engine mount path. + #[serde(default = "default_vault_kubernetes_mount")] + mount: String, + /// Projected ServiceAccount token to present. Re-read on every login so + /// a token the kubelet rotates is picked up without a restart. + #[serde(default = "default_vault_kubernetes_jwt_path")] + jwt_path: PathBuf, + /// Fail-closed margin in seconds, as on `AppRole`. Defaults to the + /// per-attempt timeout. + #[serde(default)] + refresh_safety_window_secs: Option, + }, /// Agent-managed token file (for example a Vault Agent auto-auth sink): /// the token is read from `path` and re-read periodically so a token /// rotated by the agent is picked up without a restart. @@ -520,6 +555,16 @@ impl VaultAuthMethod { } } + /// Kubernetes authentication with the default mount and projected token path. + pub fn kubernetes(role: String) -> Self { + Self::Kubernetes { + role, + mount: default_vault_kubernetes_mount(), + jwt_path: default_vault_kubernetes_jwt_path(), + refresh_safety_window_secs: None, + } + } + /// Agent-managed token file with the default poll interval. pub fn token_file(path: PathBuf) -> Self { Self::TokenFile { @@ -548,6 +593,20 @@ impl fmt::Debug for VaultAuthMethod { .field("mount", mount) .field("refresh_safety_window_secs", refresh_safety_window_secs) .finish(), + // No redaction: the role and mount name a Vault binding, and the + // ServiceAccount token itself is never held on this type. + Self::Kubernetes { + role, + mount, + jwt_path, + refresh_safety_window_secs, + } => f + .debug_struct("Kubernetes") + .field("role", role) + .field("mount", mount) + .field("jwt_path", jwt_path) + .field("refresh_safety_window_secs", refresh_safety_window_secs) + .finish(), Self::TokenFile { path, poll_interval_secs, @@ -1028,50 +1087,12 @@ impl KmsConfig { }); } KmsBackend::VaultKv2 => { - let address = get_env_str("RUSTFS_KMS_VAULT_ADDRESS", "http://localhost:8200"); - let auth_method = vault_auth_method_from_env()?; - let skip_tls_verify = get_env_bool(ENV_KMS_VAULT_SKIP_TLS_VERIFY, false); - - let mount_path = match get_env_opt_str("RUSTFS_KMS_VAULT_MOUNT_PATH") { - Some(path) => { - tracing::warn!( - "RUSTFS_KMS_VAULT_MOUNT_PATH is deprecated for the Vault KV2 backend: it never calls the Transit engine and the value is stored but unused" - ); - path - } - None => default_vault_kv2_mount_path(), - }; - - config.backend_config = BackendConfig::VaultKv2(Box::new(VaultConfig { - address, - auth_method, - namespace: get_env_opt_str("RUSTFS_KMS_VAULT_NAMESPACE"), - mount_path, - kv_mount: get_env_str("RUSTFS_KMS_VAULT_KV_MOUNT", "secret"), - key_path_prefix: get_env_str("RUSTFS_KMS_VAULT_KEY_PREFIX", "rustfs/kms/keys"), - tls: vault_tls_config(skip_tls_verify), - })); + config.backend_config = + BackendConfig::VaultKv2(Box::new(vault_kv2_config_from_env(VaultCliOverrides::default())?)); } KmsBackend::VaultTransit => { - let address = get_env_str("RUSTFS_KMS_VAULT_ADDRESS", "http://localhost:8200"); - let auth_method = vault_auth_method_from_env()?; - let skip_tls_verify = get_env_bool(ENV_KMS_VAULT_SKIP_TLS_VERIFY, false); - - config.backend_config = BackendConfig::VaultTransit(Box::new(VaultTransitConfig { - address, - auth_method, - namespace: get_env_opt_str("RUSTFS_KMS_VAULT_NAMESPACE"), - mount_path: get_env_str("RUSTFS_KMS_VAULT_MOUNT_PATH", "transit"), - metadata_kv_mount: get_env_str( - ENV_KMS_VAULT_TRANSIT_METADATA_KV_MOUNT, - DEFAULT_VAULT_TRANSIT_METADATA_KV_MOUNT, - ), - metadata_key_prefix: get_env_str( - ENV_KMS_VAULT_TRANSIT_METADATA_PREFIX, - DEFAULT_VAULT_TRANSIT_METADATA_KEY_PREFIX, - ), - tls: vault_tls_config(skip_tls_verify), - })); + config.backend_config = + BackendConfig::VaultTransit(Box::new(vault_transit_config_from_env(VaultCliOverrides::default())?)); } KmsBackend::Static => { // Read from file first, then fall back to direct env var @@ -1202,6 +1223,78 @@ fn is_under_temp_dir(path: &Path) -> bool { path.starts_with(std::env::temp_dir()) } +/// Command-line values that take precedence over the matching environment +/// variables when assembling a Vault backend configuration. +/// +/// Every field has a `RUSTFS_KMS_VAULT_*` equivalent that the CLI layer already +/// reads, so these are only set when the operator passed an explicit flag. +/// +/// Deliberately not `Debug`: `token` holds the raw Vault token, and the +/// redacting `Debug` impls elsewhere in this module exist because a derived one +/// would print it. Denying the derive makes a future `{overrides:?}` a compile +/// error instead of a leak. +#[derive(Default, Clone, Copy)] +pub struct VaultCliOverrides<'a> { + pub address: Option<&'a str>, + pub token: Option<&'a str>, + pub mount_path: Option<&'a str>, +} + +/// Assemble the Vault KV2 backend configuration from the environment. +/// +/// Shared by [`KmsConfig::from_env`] and the server's command-line startup path +/// so both resolve the same auth method, namespace, TLS and mount settings. +pub fn vault_kv2_config_from_env(overrides: VaultCliOverrides<'_>) -> Result { + let mount_path = match overrides + .mount_path + .map(str::to_string) + .or_else(|| get_env_opt_str(ENV_KMS_VAULT_MOUNT_PATH)) + { + Some(path) => { + tracing::warn!( + "RUSTFS_KMS_VAULT_MOUNT_PATH is deprecated for the Vault KV2 backend: it never calls the Transit engine and the value is stored but unused" + ); + path + } + None => default_vault_kv2_mount_path(), + }; + + Ok(VaultConfig { + address: vault_address_from_env(overrides.address), + auth_method: vault_auth_method_from_env(overrides.token)?, + namespace: get_env_opt_str(ENV_KMS_VAULT_NAMESPACE), + mount_path, + kv_mount: get_env_str("RUSTFS_KMS_VAULT_KV_MOUNT", "secret"), + key_path_prefix: get_env_str("RUSTFS_KMS_VAULT_KEY_PREFIX", "rustfs/kms/keys"), + tls: vault_tls_config(get_env_bool(ENV_KMS_VAULT_SKIP_TLS_VERIFY, false)), + }) +} + +/// Assemble the Vault Transit backend configuration from the environment. +/// +/// Companion to [`vault_kv2_config_from_env`]; see there for why both entry +/// points share it. +pub fn vault_transit_config_from_env(overrides: VaultCliOverrides<'_>) -> Result { + Ok(VaultTransitConfig { + address: vault_address_from_env(overrides.address), + auth_method: vault_auth_method_from_env(overrides.token)?, + namespace: get_env_opt_str(ENV_KMS_VAULT_NAMESPACE), + mount_path: overrides + .mount_path + .map(str::to_string) + .unwrap_or_else(|| get_env_str(ENV_KMS_VAULT_MOUNT_PATH, "transit")), + metadata_kv_mount: get_env_str(ENV_KMS_VAULT_TRANSIT_METADATA_KV_MOUNT, DEFAULT_VAULT_TRANSIT_METADATA_KV_MOUNT), + metadata_key_prefix: get_env_str(ENV_KMS_VAULT_TRANSIT_METADATA_PREFIX, DEFAULT_VAULT_TRANSIT_METADATA_KEY_PREFIX), + tls: vault_tls_config(get_env_bool(ENV_KMS_VAULT_SKIP_TLS_VERIFY, false)), + }) +} + +fn vault_address_from_env(override_value: Option<&str>) -> String { + override_value + .map(str::to_string) + .unwrap_or_else(|| get_env_str(ENV_KMS_VAULT_ADDRESS, "http://localhost:8200")) +} + /// Resolve the Vault auth method from environment variables. /// /// Setting `RUSTFS_KMS_VAULT_APPROLE_ROLE_ID` selects AppRole authentication; @@ -1209,27 +1302,59 @@ fn is_under_temp_dir(path: &Path) -> bool { /// (re-read on every login, mirroring the `RUSTFS_KMS_STATIC_SECRET_KEY_FILE` /// precedent) or inline from `RUSTFS_KMS_VAULT_APPROLE_SECRET_ID`, with the /// file taking precedence. Without a role id the legacy token flow applies. -fn vault_auth_method_from_env() -> Result { +/// +/// `RUSTFS_KMS_VAULT_KUBERNETES_ROLE` selects Kubernetes authentication, which +/// presents the pod's projected ServiceAccount token. +/// +/// `token_override` carries a token supplied on the command line; it stands in +/// for `RUSTFS_KMS_VAULT_TOKEN` everywhere below, including the conflict checks, +/// so a flag and the variable it mirrors select the same method. +fn vault_auth_method_from_env(token_override: Option<&str>) -> Result { + let token = token_override + .map(str::to_string) + .or_else(|| get_env_opt_str(ENV_KMS_VAULT_TOKEN)); + let role_id = get_env_opt_str(ENV_KMS_VAULT_APPROLE_ROLE_ID); + let kubernetes_role = get_env_opt_str(ENV_KMS_VAULT_KUBERNETES_ROLE); + if let Some(token_file) = get_env_opt_str(ENV_KMS_VAULT_TOKEN_FILE) { // A token file names one authoritative credential source; combining it // with another one would leave the effective identity ambiguous, so // that is a configuration error rather than a precedence rule. - if get_env_opt_str(ENV_KMS_VAULT_APPROLE_ROLE_ID).is_some() { - return Err(KmsError::configuration_error(format!( - "{ENV_KMS_VAULT_TOKEN_FILE} cannot be combined with {ENV_KMS_VAULT_APPROLE_ROLE_ID}; configure exactly one Vault auth method" - ))); - } - if get_env_opt_str("RUSTFS_KMS_VAULT_TOKEN").is_some() { - return Err(KmsError::configuration_error(format!( - "{ENV_KMS_VAULT_TOKEN_FILE} cannot be combined with RUSTFS_KMS_VAULT_TOKEN; configure exactly one Vault auth method" - ))); + for (name, configured) in [ + (ENV_KMS_VAULT_APPROLE_ROLE_ID, role_id.is_some()), + (ENV_KMS_VAULT_KUBERNETES_ROLE, kubernetes_role.is_some()), + (ENV_KMS_VAULT_TOKEN, token.is_some()), + ] { + if configured { + return Err(KmsError::configuration_error(format!( + "{ENV_KMS_VAULT_TOKEN_FILE} cannot be combined with {name}; configure exactly one Vault auth method" + ))); + } } return Ok(VaultAuthMethod::token_file(PathBuf::from(token_file))); } - let Some(role_id) = get_env_opt_str(ENV_KMS_VAULT_APPROLE_ROLE_ID) else { + if let Some(role) = kubernetes_role { + // Unlike a leftover static token, a second login method is never a + // stale remnant: both were configured deliberately and neither can be + // ranked over the other. + if role_id.is_some() { + return Err(KmsError::configuration_error(format!( + "{ENV_KMS_VAULT_KUBERNETES_ROLE} cannot be combined with {ENV_KMS_VAULT_APPROLE_ROLE_ID}; configure exactly one Vault auth method" + ))); + } + return Ok(VaultAuthMethod::Kubernetes { + role, + mount: get_env_str(ENV_KMS_VAULT_KUBERNETES_MOUNT, DEFAULT_VAULT_KUBERNETES_MOUNT), + jwt_path: get_env_opt_str(ENV_KMS_VAULT_KUBERNETES_JWT_PATH) + .map_or_else(default_vault_kubernetes_jwt_path, PathBuf::from), + refresh_safety_window_secs: None, + }); + } + + let Some(role_id) = role_id else { return Ok(VaultAuthMethod::Token { - token: get_env_str("RUSTFS_KMS_VAULT_TOKEN", "dev-token"), + token: token.unwrap_or_else(|| "dev-token".to_string()), }); }; @@ -1273,6 +1398,22 @@ fn validate_vault_auth_method(backend_name: &str, auth_method: &VaultAuthMethod) } Ok(()) } + VaultAuthMethod::Kubernetes { + role, mount, jwt_path, .. + } => { + if role.is_empty() { + return Err(KmsError::configuration_error(format!("{backend_name} Kubernetes role cannot be empty"))); + } + if mount.is_empty() { + return Err(KmsError::configuration_error(format!("{backend_name} Kubernetes mount cannot be empty"))); + } + if jwt_path.as_os_str().is_empty() { + return Err(KmsError::configuration_error(format!( + "{backend_name} Kubernetes ServiceAccount token path cannot be empty" + ))); + } + Ok(()) + } VaultAuthMethod::TokenFile { path, poll_interval_secs, @@ -1976,6 +2117,106 @@ mod tests { .expect("well-formed token file auth must validate"); } + /// A Kubernetes role alone configures the method: the credential is the + /// pod's projected ServiceAccount token, so nothing secret is in the + /// environment and the mount and token path fall back to the cluster + /// defaults. + #[test] + fn test_from_env_selects_kubernetes() { + with_vars( + vec![ + ("RUSTFS_KMS_BACKEND", Some("vault-transit")), + (ENV_KMS_VAULT_ADDRESS, Some("https://vault.example.com")), + (ENV_KMS_VAULT_KUBERNETES_ROLE, Some("rustfs")), + (ENV_KMS_VAULT_KUBERNETES_MOUNT, None), + (ENV_KMS_VAULT_KUBERNETES_JWT_PATH, None), + (ENV_KMS_VAULT_TOKEN, None), + (ENV_KMS_VAULT_TOKEN_FILE, None), + (ENV_KMS_VAULT_APPROLE_ROLE_ID, None), + ], + || { + let config = KmsConfig::from_env().expect("kms config should load from env"); + let vault = config.vault_transit_config().expect("vault transit backend config"); + let VaultAuthMethod::Kubernetes { + role, + mount, + jwt_path, + refresh_safety_window_secs, + } = &vault.auth_method + else { + panic!( + "a kubernetes role in the environment must select Kubernetes auth, got {:?}", + vault.auth_method + ); + }; + assert_eq!(role, "rustfs"); + assert_eq!(mount, DEFAULT_VAULT_KUBERNETES_MOUNT); + assert_eq!(jwt_path, Path::new(DEFAULT_VAULT_KUBERNETES_JWT_PATH)); + assert_eq!(refresh_safety_window_secs, &None); + }, + ); + } + + #[test] + fn test_from_env_kubernetes_is_mutually_exclusive_with_other_auth() { + with_vars( + vec![ + ("RUSTFS_KMS_BACKEND", Some("vault-transit")), + (ENV_KMS_VAULT_KUBERNETES_ROLE, Some("rustfs")), + (ENV_KMS_VAULT_APPROLE_ROLE_ID, Some("env-role-id")), + (ENV_KMS_VAULT_TOKEN, None), + (ENV_KMS_VAULT_TOKEN_FILE, None), + ], + || { + let error = KmsConfig::from_env().expect_err("kubernetes combined with approle must be rejected"); + assert!(error.to_string().contains(ENV_KMS_VAULT_KUBERNETES_ROLE)); + assert!(error.to_string().contains(ENV_KMS_VAULT_APPROLE_ROLE_ID)); + }, + ); + } + + #[test] + fn test_validate_rejects_bad_kubernetes_settings() { + let vault_config = |auth_method: VaultAuthMethod| KmsConfig { + backend: KmsBackend::VaultTransit, + backend_config: BackendConfig::VaultTransit(Box::new(VaultTransitConfig { + address: "https://vault.example.com:8200".to_string(), + auth_method, + ..Default::default() + })), + ..Default::default() + }; + + let error = vault_config(VaultAuthMethod::kubernetes(String::new())) + .validate() + .expect_err("an empty kubernetes role must be rejected"); + assert!(error.to_string().contains("role"), "got {error}"); + + let error = vault_config(VaultAuthMethod::Kubernetes { + role: "rustfs".to_string(), + mount: String::new(), + jwt_path: PathBuf::from(DEFAULT_VAULT_KUBERNETES_JWT_PATH), + refresh_safety_window_secs: None, + }) + .validate() + .expect_err("an empty kubernetes mount must be rejected"); + assert!(error.to_string().contains("mount"), "got {error}"); + + let error = vault_config(VaultAuthMethod::Kubernetes { + role: "rustfs".to_string(), + mount: DEFAULT_VAULT_KUBERNETES_MOUNT.to_string(), + jwt_path: PathBuf::new(), + refresh_safety_window_secs: None, + }) + .validate() + .expect_err("an empty ServiceAccount token path must be rejected"); + assert!(error.to_string().contains("token path"), "got {error}"); + + vault_config(VaultAuthMethod::kubernetes("rustfs".to_string())) + .validate() + .expect("well-formed kubernetes auth must validate"); + } + /// Every KV2 read, write and listing is routed through `kv_mount`, so an /// empty one names a path no Vault engine answers. The Transit backend /// already rejects its own empty mounts; this closes the same gap on the diff --git a/docs/operations/kms-backend-security.md b/docs/operations/kms-backend-security.md index 4fe4aad07..3f1dfdc7e 100644 --- a/docs/operations/kms-backend-security.md +++ b/docs/operations/kms-backend-security.md @@ -2,7 +2,7 @@ RustFS ships several KMS backends. They differ not only in deployment effort but in **where master key material lives and who can read it**. Pick a backend based on the confidentiality boundary you need, not on the name alone. -For how the Vault backends authenticate (static token, AppRole, Vault Agent token file) and how credential refresh and the fail-closed window behave, see the [Vault KMS authentication runbook](vault-kms-authentication.md). For what may be claimed about the cryptographic implementations themselves, see [Cryptographic compliance positioning](kms-cryptographic-compliance.md). For which RustFS identities may manage or use a given key, see [Per-key KMS authorization](kms-per-key-authorization.md). If you are migrating from MinIO, read [Migrating from MinIO: encrypted objects do not carry over](#migrating-from-minio-encrypted-objects-do-not-carry-over) first. +For how the Vault backends authenticate (static token, AppRole, Kubernetes, Vault Agent token file) and how credential refresh and the fail-closed window behave, see the [Vault KMS authentication runbook](vault-kms-authentication.md). For what may be claimed about the cryptographic implementations themselves, see [Cryptographic compliance positioning](kms-cryptographic-compliance.md). For which RustFS identities may manage or use a given key, see [Per-key KMS authorization](kms-per-key-authorization.md). If you are migrating from MinIO, read [Migrating from MinIO: encrypted objects do not carry over](#migrating-from-minio-encrypted-objects-do-not-carry-over) first. ## Backend comparison diff --git a/docs/operations/vault-kms-authentication.md b/docs/operations/vault-kms-authentication.md index 6ee0a0c2c..adabd2918 100644 --- a/docs/operations/vault-kms-authentication.md +++ b/docs/operations/vault-kms-authentication.md @@ -8,9 +8,12 @@ This runbook covers how the RustFS Vault KMS backends (KV2 and Transit) authenti | --- | --- | --- | --- | --- | | Static token | `Token` | Whatever the operator provisioned; RustFS never renews it | None | Development; short-lived experiments | | AppRole | `AppRole` | Lease-bound token obtained by login; renewed by RustFS | Renew at half TTL, re-login on failure | Production without a Vault Agent sidecar | +| Kubernetes | `Kubernetes` | Lease-bound token obtained by login; renewed by RustFS | Renew at half TTL, re-login on failure | Production on Kubernetes, with no credential to distribute | | Agent token file | `TokenFile` | Owned by Vault Agent; RustFS only re-reads the sink file | File re-read once per poll interval | Production with a Vault Agent (or equivalent) managing auth | -Exactly one method must be configured. Setting `RUSTFS_KMS_VAULT_TOKEN_FILE` together with `RUSTFS_KMS_VAULT_APPROLE_ROLE_ID` or an explicit `RUSTFS_KMS_VAULT_TOKEN` is rejected at startup with a configuration error, because the effective identity would be ambiguous. +Exactly one method must be configured. Setting `RUSTFS_KMS_VAULT_TOKEN_FILE` together with any other method, or `RUSTFS_KMS_VAULT_KUBERNETES_ROLE` together with `RUSTFS_KMS_VAULT_APPROLE_ROLE_ID`, is rejected at startup with a configuration error, because the effective identity would be ambiguous. A leftover `RUSTFS_KMS_VAULT_TOKEN` alongside a configured login method is tolerated and ignored, so a stale variable cannot silently downgrade the identity. + +All of these are read the same way whether the service is started with `RUSTFS_KMS_ENABLE=true` or configured later through `POST /rustfs/admin/v3/kms/configure`. The default `dev-token` fallback for `RUSTFS_KMS_VAULT_TOKEN` is rejected outside explicit development mode (`RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true`), as are plain-HTTP Vault addresses and disabled TLS verification. @@ -56,7 +59,44 @@ Deliver the SecretID out of band — a secrets-manager-mounted file, an init-con The secret_id file is re-read on every login attempt, so rotating the SecretID is a two-step operation with no restart: generate a new SecretID (`vault write -f auth/approle/role/rustfs-kms/secret-id`), atomically replace the file, then revoke the old SecretID accessor. The already-issued token keeps renewing; the new SecretID is only needed at the next full re-login. -An empty or missing secret_id file fails the login attempt immediately (no Vault round trip) and is retried on the normal refresh cadence, so repairing the file heals the backend without a restart. +An empty or missing secret_id file fails the login attempt immediately (no Vault round trip). At startup the error is fatal — provider construction fails and the process exits — so a file missing at boot is recovered by restarting the process, not by an in-process retry. Once RustFS is running, the same failure is retried on the normal refresh cadence, so repairing the file mid-run heals the backend without a restart. + +## Kubernetes authentication + +On Kubernetes this is the method to prefer: the pod's own ServiceAccount is the identity, so there is no credential to distribute, rotate, or leak into a Secret. + +### Vault-side setup + +```shell +vault auth enable kubernetes + +vault write auth/kubernetes/config \ + kubernetes_host="https://$KUBERNETES_SERVICE_HOST:$KUBERNETES_SERVICE_PORT" + +vault write auth/kubernetes/role/rustfs \ + bound_service_account_names=rustfs \ + bound_service_account_namespaces=rustfs \ + token_policies=rustfs-kms \ + token_ttl=1h +``` + +As with AppRole, keep `token_ttl` comfortably above the RustFS per-attempt timeout (default 30s). + +### RustFS configuration + +```shell +RUSTFS_KMS_BACKEND=vault-transit # or "vault" for the KV2 backend +RUSTFS_KMS_VAULT_ADDRESS=https://vault.vault.svc.cluster.local:8200 +RUSTFS_KMS_VAULT_KUBERNETES_ROLE=rustfs +# Optional, defaults to "kubernetes": +# RUSTFS_KMS_VAULT_KUBERNETES_MOUNT=kubernetes +# Optional, defaults to the kubelet's projected token path: +# RUSTFS_KMS_VAULT_KUBERNETES_JWT_PATH=/var/run/secrets/kubernetes.io/serviceaccount/token +``` + +RustFS logs in at startup and renews the token at half its TTL, falling back to a fresh login exactly as AppRole does. The ServiceAccount token is re-read from disk on every login rather than cached, so a projected token the kubelet rotates is picked up without a restart. + +A missing or empty token file fails the login attempt immediately (no Vault round trip). At startup the error is fatal — provider construction fails and the process exits — so a token projected late during a slow pod start is recovered by the pod restart loop, not by an in-process retry. Once RustFS is running, a token file that goes missing or turns empty is retried on the normal refresh cadence and heals the backend on its own. ## Vault Agent token file @@ -101,13 +141,13 @@ If the agent stops refreshing the file that is fine — RustFS re-reads the same ## Fail-closed window -For lease-bound credentials (AppRole tokens, token files), `current()` refuses to hand out a token that is within the safety window of its expiry and has not been refreshed. Requests then fail with `KMS credentials unavailable: ...` instead of being sent with a token that could lapse mid-flight and fail unpredictably on the Vault side. +For lease-bound credentials (AppRole and Kubernetes tokens, token files), `current()` refuses to hand out a token that is within the safety window of its expiry and has not been refreshed. Requests then fail with `KMS credentials unavailable: ...` instead of being sent with a token that could lapse mid-flight and fail unpredictably on the Vault side. - Default window: one per-attempt timeout (`RUSTFS_KMS_TIMEOUT_SECS`, default 30s) — a request issued now can legitimately stay in flight that long, so the token must outlive it. -- Override: `refresh_safety_window_secs` on the `AppRole` or `TokenFile` auth configuration. +- Override: `refresh_safety_window_secs` on the `AppRole`, `Kubernetes` or `TokenFile` auth configuration. - Static tokens never trip the window: they carry no lease and are assumed valid until Vault says otherwise. -The window is a symptom threshold, not the fault itself: by the time it trips, refresh has been failing for roughly half the token TTL (AppRole) or two poll intervals (token file). +The window is a symptom threshold, not the fault itself: by the time it trips, refresh has been failing for roughly half the token TTL (AppRole, Kubernetes) or two poll intervals (token file). ### Troubleshooting @@ -117,6 +157,8 @@ The window is a symptom threshold, not the fault itself: by the time it trips, r | Renewal succeeded but re-login later fails | `Vault token renewal failed; falling back to a fresh login` followed by login errors | SecretID expired/revoked or AppRole role changed; rotate the secret_id file | | Token file mode error at startup or during polls | `has insecure permissions` in the error | Fix the sink `mode` (0600) and the file owner; the next poll heals the provider | | Token file missing/empty errors | `Failed to read Vault token file` / `token file ... is empty` | Vault Agent down or sink misconfigured; restart the agent, the next poll heals the provider | -| Startup fails immediately with a configuration error naming two env vars | — | Two auth methods configured at once; keep exactly one of token, AppRole, token file | +| Kubernetes login fails with a permission error | `Vault Kubernetes login failed` | The pod's ServiceAccount is not in the role's `bound_service_account_names`/`_namespaces`, or `auth/kubernetes/config` names the wrong API server | +| Kubernetes ServiceAccount token errors | `Failed to read Kubernetes ServiceAccount token` / `ServiceAccount token ... is empty` | The token is not projected into the pod (check `automountServiceAccountToken` and the volume mount); the next refresh cycle heals the provider | +| Startup fails immediately with a configuration error naming two env vars | — | Two auth methods configured at once; keep exactly one of token, AppRole, Kubernetes, token file | When diagnosing, confirm three clocks/lifetimes in order: the Vault token TTL (`vault token lookup` with the token's accessor), the RustFS refresh cadence (half TTL or the poll interval), and the fail-closed window. The renewal task logs every failed cycle, so a silent gap in warnings combined with `CredentialsUnavailable` errors points at the process clock or a paused runtime rather than Vault. diff --git a/rustfs/src/admin/handlers/kms_backup.rs b/rustfs/src/admin/handlers/kms_backup.rs index 02384f487..02682e7da 100644 --- a/rustfs/src/admin/handlers/kms_backup.rs +++ b/rustfs/src/admin/handlers/kms_backup.rs @@ -286,6 +286,7 @@ fn auth_method_kind(auth: &VaultAuthMethod) -> String { match auth { VaultAuthMethod::Token { .. } => "token", VaultAuthMethod::AppRole { .. } => "approle", + VaultAuthMethod::Kubernetes { .. } => "kubernetes", VaultAuthMethod::TokenFile { .. } => "token-file", } .to_string() @@ -484,7 +485,10 @@ fn business_trust_root_secrets(config: &KmsConfig) -> Vec> { secrets.push(Zeroizing::new(role_id.clone())); secrets.push(Zeroizing::new(secret_id.clone())); } - VaultAuthMethod::TokenFile { .. } => {} + // Kubernetes and TokenFile hold no inline plaintext credential: the + // ServiceAccount token and the agent-managed token live in files, and + // the role names a Vault binding rather than half a credential pair. + VaultAuthMethod::Kubernetes { .. } | VaultAuthMethod::TokenFile { .. } => {} }; match &config.backend_config { diff --git a/rustfs/src/init.rs b/rustfs/src/init.rs index 67c889c1a..34a1d6ca0 100644 --- a/rustfs/src/init.rs +++ b/rustfs/src/init.rs @@ -304,30 +304,37 @@ fn build_local_kms_config(cfg: &config::Config) -> std::io::Result( + cfg: &'a config::Config, + backend_name: &str, +) -> std::io::Result> { + let address = cfg + .kms_vault_address + .as_deref() + .ok_or_else(|| Error::other(format!("Vault address is required for {backend_name} backend")))?; + + Ok(rustfs_kms::config::VaultCliOverrides { + address: Some(address), + token: cfg.kms_vault_token.as_deref(), + mount_path: cfg.kms_vault_mount_path.as_deref(), + }) +} + /// Build KMS configuration for Vault backend fn build_vault_kms_config(cfg: &config::Config) -> std::io::Result { - let vault_address = cfg - .kms_vault_address - .as_ref() - .ok_or_else(|| Error::other("Vault address is required for vault backend"))?; - let vault_token = cfg - .kms_vault_token - .as_ref() - .ok_or_else(|| Error::other("Vault token is required for vault backend"))?; + let backend_config = rustfs_kms::config::vault_kv2_config_from_env(vault_cli_overrides(cfg, "vault")?) + .map_err(|e| Error::other(format!("Vault KMS configuration failed: {e}")))?; let kms_config = rustfs_kms::config::KmsConfig { backend: rustfs_kms::config::KmsBackend::VaultKv2, - backend_config: rustfs_kms::config::BackendConfig::VaultKv2(Box::new(rustfs_kms::config::VaultConfig { - address: vault_address.clone(), - auth_method: rustfs_kms::config::VaultAuthMethod::Token { - token: vault_token.clone(), - }, - namespace: None, - mount_path: cfg.kms_vault_mount_path.clone().unwrap_or_else(|| "transit".to_string()), - kv_mount: "secret".to_string(), - key_path_prefix: "rustfs/kms/keys".to_string(), - tls: None, - })), + backend_config: rustfs_kms::config::BackendConfig::VaultKv2(Box::new(backend_config)), allow_insecure_dev_defaults: cfg.kms_allow_insecure_dev_defaults, allow_immediate_deletion: rustfs_kms::config::allow_immediate_deletion_from_env(), default_key_id: cfg.kms_default_key_id.clone(), @@ -344,26 +351,12 @@ fn build_vault_kms_config(cfg: &config::Config) -> std::io::Result std::io::Result { - let vault_address = cfg - .kms_vault_address - .as_ref() - .ok_or_else(|| Error::other("Vault address is required for vault-transit backend"))?; - let vault_token = cfg - .kms_vault_token - .as_ref() - .ok_or_else(|| Error::other("Vault token is required for vault-transit backend"))?; + let backend_config = rustfs_kms::config::vault_transit_config_from_env(vault_cli_overrides(cfg, "vault-transit")?) + .map_err(|e| Error::other(format!("Vault Transit KMS configuration failed: {e}")))?; let kms_config = rustfs_kms::config::KmsConfig { backend: rustfs_kms::config::KmsBackend::VaultTransit, - backend_config: rustfs_kms::config::BackendConfig::VaultTransit(Box::new(rustfs_kms::config::VaultTransitConfig { - address: vault_address.clone(), - auth_method: rustfs_kms::config::VaultAuthMethod::Token { - token: vault_token.clone(), - }, - namespace: None, - mount_path: cfg.kms_vault_mount_path.clone().unwrap_or_else(|| "transit".to_string()), - ..rustfs_kms::config::VaultTransitConfig::default() - })), + backend_config: rustfs_kms::config::BackendConfig::VaultTransit(Box::new(backend_config)), allow_insecure_dev_defaults: cfg.kms_allow_insecure_dev_defaults, allow_immediate_deletion: rustfs_kms::config::allow_immediate_deletion_from_env(), default_key_id: cfg.kms_default_key_id.clone(), @@ -1405,7 +1398,10 @@ pub async fn init_sftp_system() -> Result, Box crate::config::Config { + let mut config = crate::config::Config::new("127.0.0.1:9000", vec!["/tmp/rustfs-vault-kms".to_string()]); + config.kms_enable = true; + config.kms_backend = backend.to_string(); + config.kms_vault_address = Some("https://vault.example.com:8200".to_string()); + config + } + + /// The Vault auth method and the settings the CLI has no flag for come from + /// the environment, so startup and `KmsConfig::from_env` cannot disagree. + /// Regression: startup used to hardcode token auth and require a token, + /// which made every non-token method unreachable through `RUSTFS_KMS_ENABLE`. + #[test] + fn build_vault_transit_kms_config_resolves_auth_and_mounts_from_env() { + let config = temp_env::with_vars( + [ + ("RUSTFS_KMS_VAULT_TOKEN", None), + ("RUSTFS_KMS_VAULT_TOKEN_FILE", None), + ("RUSTFS_KMS_VAULT_KUBERNETES_ROLE", None), + ("RUSTFS_KMS_VAULT_APPROLE_ROLE_ID", Some("env-role-id")), + ("RUSTFS_KMS_VAULT_APPROLE_SECRET_ID", Some("env-secret-id")), + ("RUSTFS_KMS_VAULT_APPROLE_SECRET_ID_FILE", None), + ("RUSTFS_KMS_VAULT_NAMESPACE", Some("team-a")), + ("RUSTFS_KMS_VAULT_TRANSIT_METADATA_KV_MOUNT", Some("rustfs-kv")), + ], + || { + build_vault_transit_kms_config(&vault_kms_test_config("vault-transit")) + .expect("vault transit KMS configuration should build") + }, + ); + + let vault = config.vault_transit_config().expect("vault transit backend config"); + let rustfs_kms::config::VaultAuthMethod::AppRole { role_id, secret_id, .. } = &vault.auth_method else { + panic!("approle in the environment must select AppRole auth, got {:?}", vault.auth_method); + }; + assert_eq!(role_id, "env-role-id"); + assert_eq!(secret_id, "env-secret-id"); + assert_eq!(vault.namespace.as_deref(), Some("team-a")); + assert_eq!(vault.metadata_kv_mount, "rustfs-kv"); + } + + /// Kubernetes auth needs no credential in the environment at all: the role + /// selects it and the pod's projected ServiceAccount token supplies the rest. + #[test] + fn build_vault_transit_kms_config_selects_kubernetes_auth() { + let config = temp_env::with_vars( + [ + ("RUSTFS_KMS_VAULT_TOKEN", None), + ("RUSTFS_KMS_VAULT_TOKEN_FILE", None), + ("RUSTFS_KMS_VAULT_APPROLE_ROLE_ID", None), + ("RUSTFS_KMS_VAULT_KUBERNETES_ROLE", Some("rustfs")), + ("RUSTFS_KMS_VAULT_KUBERNETES_MOUNT", None), + ("RUSTFS_KMS_VAULT_KUBERNETES_JWT_PATH", None), + ], + || { + build_vault_transit_kms_config(&vault_kms_test_config("vault-transit")) + .expect("vault transit KMS configuration should build") + }, + ); + + let vault = config.vault_transit_config().expect("vault transit backend config"); + let rustfs_kms::config::VaultAuthMethod::Kubernetes { + role, mount, jwt_path, .. + } = &vault.auth_method + else { + panic!( + "a kubernetes role in the environment must select Kubernetes auth, got {:?}", + vault.auth_method + ); + }; + assert_eq!(role, "rustfs"); + assert_eq!(mount, rustfs_kms::config::DEFAULT_VAULT_KUBERNETES_MOUNT); + assert_eq!(jwt_path, std::path::Path::new(rustfs_kms::config::DEFAULT_VAULT_KUBERNETES_JWT_PATH)); + } + + /// Two credential sources leave the effective identity ambiguous, so + /// startup refuses rather than picking one. + #[test] + fn build_vault_kms_config_refuses_two_auth_methods() { + temp_env::with_vars( + [ + ("RUSTFS_KMS_VAULT_TOKEN", None), + ("RUSTFS_KMS_VAULT_TOKEN_FILE", Some("/run/vault-agent/token")), + ("RUSTFS_KMS_VAULT_APPROLE_ROLE_ID", None), + ("RUSTFS_KMS_VAULT_KUBERNETES_ROLE", Some("rustfs")), + ], + || { + let error = build_vault_kms_config(&vault_kms_test_config("vault")) + .expect_err("two Vault auth methods must not start the server"); + assert!(error.to_string().contains("exactly one"), "unexpected error: {error}"); + }, + ); + } + + /// The KV2 backend has its own builder, so the key-location settings have + /// to be proven separately from the Transit one: pointing at the wrong KV + /// mount or prefix makes existing keys look absent. + #[test] + fn build_vault_kms_config_resolves_kv_mount_and_prefix_from_env() { + let config = temp_env::with_vars( + [ + ("RUSTFS_KMS_VAULT_TOKEN", Some("a-real-token")), + ("RUSTFS_KMS_VAULT_TOKEN_FILE", None), + ("RUSTFS_KMS_VAULT_APPROLE_ROLE_ID", None), + ("RUSTFS_KMS_VAULT_KUBERNETES_ROLE", None), + ("RUSTFS_KMS_VAULT_KV_MOUNT", Some("rustfs-kv")), + ("RUSTFS_KMS_VAULT_KEY_PREFIX", Some("tenant/keys")), + ], + || build_vault_kms_config(&vault_kms_test_config("vault")).expect("vault KV2 KMS configuration should build"), + ); + + let vault = config.vault_config().expect("vault kv2 backend config"); + assert_eq!(vault.kv_mount, "rustfs-kv"); + assert_eq!(vault.key_path_prefix, "tenant/keys"); + } + + /// Skipping TLS verification was silently dropped on this path before, so + /// an operator who asked for it still got a verified connection. Now that it + /// is honoured it must fail closed without the development opt-in, rather + /// than quietly downgrading the Vault connection. + #[test] + fn build_vault_transit_kms_config_refuses_skip_tls_verify_without_opt_in() { + let vars = [ + ("RUSTFS_KMS_VAULT_TOKEN", Some("a-real-token")), + ("RUSTFS_KMS_VAULT_TOKEN_FILE", None), + ("RUSTFS_KMS_VAULT_APPROLE_ROLE_ID", None), + ("RUSTFS_KMS_VAULT_KUBERNETES_ROLE", None), + ("RUSTFS_KMS_VAULT_SKIP_TLS_VERIFY", Some("true")), + ]; + + temp_env::with_vars(vars, || { + let error = build_vault_transit_kms_config(&vault_kms_test_config("vault-transit")) + .expect_err("skipping TLS verification must not start the server"); + assert!(error.to_string().contains("TLS"), "unexpected error: {error}"); + }); + + temp_env::with_vars(vars, || { + let mut cfg = vault_kms_test_config("vault-transit"); + cfg.kms_allow_insecure_dev_defaults = true; + let config = build_vault_transit_kms_config(&cfg).expect("the development opt-in should accept skip-verify"); + let vault = config.vault_transit_config().expect("vault transit backend config"); + assert!(vault.tls.as_ref().is_some_and(|tls| tls.skip_verify)); + }); + } + fn aws_kms_test_config() -> crate::config::Config { let mut config = crate::config::Config::new("127.0.0.1:9000", vec!["/tmp/rustfs-aws-kms".to_string()]); config.kms_enable = true; From 89e25132055efa3f316b237b404966cc52364fdb Mon Sep 17 00:00:00 2001 From: houseme Date: Mon, 17 Aug 2026 15:04:23 +0800 Subject: [PATCH 06/10] feat(ecstore): pin bitrot algorithms with a startup self-test (HS-11) (#6165) feat(ecstore): pin bitrot algorithms with a startup self-test A drifted HighwayHash implementation fails silently: every shard reads back corrupt, heal rewrites healthy data, and cross-platform clusters disagree about which copy is good. Mirror MinIO's bitrotSelfTest by verifying, once at process start: - known-answer digests for HighwayHash256S / HighwayHash256SLegacy over a deterministic 4096-byte xorshift64* payload, plus the externally verifiable FIPS SHA-256 "abc" vector guarding the HashAlgorithm plumbing itself; - an end-to-end roundtrip per streaming variant (encode -> size formula -> bitrot_verify -> BitrotReader read-back), over full blocks and a partial tail; - tamper detection: one flipped byte in the final data block and one in the leading hash must both be rejected as a hash mismatch, not by an incidental read error. The check costs microseconds and runs inline in init_background_service_runtime before any shard can be written or verified. Outcome surfaces as one structured bitrot_selftest log event, the rustfs_bitrot_selftest_status gauge (1=passed / 0=failed / 2=skipped), a bitrotSelftest field on the admin server-info response, and RUSTFS_BITROT_SELFTEST_STRICT=on turns a failure into a startup error (MinIO Fatal parity; the default only degrades the status so a bad build cannot brick an existing fleet on upgrade). Closes rustfs/backlog#1873 (HS-11). Co-authored-by: heihutu --- crates/ecstore/src/api/mod.rs | 4 +- crates/ecstore/src/erasure/coding/bitrot.rs | 303 +++++++++++++++++++- rustfs/src/admin/handlers/system.rs | 29 ++ rustfs/src/bitrot_selftest.rs | 181 ++++++++++++ rustfs/src/lib.rs | 1 + rustfs/src/module_switches.rs | 14 + rustfs/src/startup_background.rs | 11 +- rustfs/src/storage/storage_api.rs | 4 + rustfs/src/storage_api.rs | 4 +- 9 files changed, 535 insertions(+), 16 deletions(-) create mode 100644 rustfs/src/bitrot_selftest.rs diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index f18da095a..5526b7d00 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -373,8 +373,8 @@ pub mod error { pub mod erasure { pub use crate::erasure::coding::{ - BitrotReader, BitrotWriter, BitrotWriterWrapper, CustomWriter, Erasure, ErasureConstructionError, ReedSolomonEncoder, - calc_shard_size, calc_shard_size_legacy, + BitrotReader, BitrotSelfTestError, BitrotWriter, BitrotWriterWrapper, CustomWriter, Erasure, ErasureConstructionError, + ReedSolomonEncoder, bitrot_self_test, calc_shard_size, calc_shard_size_legacy, }; } diff --git a/crates/ecstore/src/erasure/coding/bitrot.rs b/crates/ecstore/src/erasure/coding/bitrot.rs index 7b3166fbc..698b1b02c 100644 --- a/crates/ecstore/src/erasure/coding/bitrot.rs +++ b/crates/ecstore/src/erasure/coding/bitrot.rs @@ -820,10 +820,263 @@ impl BitrotWriterWrapper { } } +// --- startup bitrot self-test (rustfs/backlog#1873, MinIO bitrotSelfTest parity) --- +// +// A broken hash implementation (bad SIMD feature combination, platform drift, a +// key-handling regression) fails silently: every shard reads back "corrupt", +// heal rewrites data that was fine, and cross-platform clusters disagree about +// which copy is healthy. The self-test below pins the algorithms the moment a +// process starts, so a drifted build announces itself instead of quietly +// rewriting objects. See docs/rustfs-heal-scanner-vs-minio-comprehensive- +// analysis-2026-08-16.md §6 HS-11. + +/// Length of the deterministic self-test payload. +pub const BITROT_SELF_TEST_PAYLOAD_LEN: usize = 4096; + +/// Known-answer digest of [`bitrot_self_test_payload`] under `HighwayHash256S` +/// (the production default). Pinned so any platform or build where the +/// implementation drifts fails startup instead of mis-hashing shards. +const BITROT_SELF_TEST_KAT_HIGHWAY_HASH256S: [u8; 32] = [ + 0xb9, 0x32, 0xa2, 0xaa, 0x4a, 0xb7, 0x33, 0x6a, 0xa3, 0xca, 0x7e, 0x61, 0x9d, 0x86, 0x52, 0x14, 0x6e, 0x7f, 0xd8, 0x9e, 0xea, + 0x08, 0xd9, 0x8c, 0x33, 0x85, 0x87, 0x19, 0x30, 0xd6, 0xed, 0x06, +]; + +/// Known-answer digest of the same payload under `HighwayHash256SLegacy`. +const BITROT_SELF_TEST_KAT_HIGHWAY_HASH256S_LEGACY: [u8; 32] = [ + 0x98, 0x24, 0x71, 0x4f, 0x16, 0xbb, 0x48, 0x39, 0xed, 0x68, 0xfa, 0x63, 0x5e, 0xd9, 0x07, 0x61, 0xdf, 0x0a, 0xff, 0xcf, 0x7d, + 0x8c, 0xa8, 0xc7, 0xc0, 0xb6, 0x6f, 0x05, 0xdb, 0xda, 0x5a, 0x22, +]; + +/// FIPS 180-2 test vector: SHA-256 of the ASCII string "abc". Unlike the +/// Highway digests above this one is externally verifiable, so it guards the +/// whole `HashAlgorithm` plumbing even for readers who distrust pinned +/// self-computed constants. +const BITROT_SELF_TEST_KAT_SHA256_ABC: [u8; 32] = [ + 0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde, 0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, + 0x17, 0x7a, 0x9c, 0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad, +]; + +/// Deterministic self-test payload: xorshift64* from a fixed seed, so every +/// platform and every run hashes the same 4096 bytes. +fn bitrot_self_test_payload() -> [u8; BITROT_SELF_TEST_PAYLOAD_LEN] { + let mut state = 0x9E37_79B9_7F4A_7C15u64; + let mut payload = [0u8; BITROT_SELF_TEST_PAYLOAD_LEN]; + for byte in payload.iter_mut() { + state ^= state >> 12; + state ^= state << 25; + state ^= state >> 27; + *byte = state.wrapping_mul(0x2545_F491_4F6C_DD1D) as u8; + } + payload +} + +/// Why a bitrot self-test failed. +#[derive(Debug)] +pub enum BitrotSelfTestError { + /// A known-answer digest mismatched the pinned constant. + KnownAnswerMismatch { + algorithm: &'static str, + got: String, + want: String, + }, + /// A freshly encoded shard failed `bitrot_verify`. + RoundtripVerify { algorithm: &'static str, detail: String }, + /// A verified roundtrip read back different bytes than were written. + RoundtripReadback { algorithm: &'static str }, + /// A deliberately tampered shard was not rejected by `bitrot_verify`. + TamperNotRejected { + algorithm: &'static str, + tampered: &'static str, + }, +} + +impl std::fmt::Display for BitrotSelfTestError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::KnownAnswerMismatch { algorithm, got, want } => { + write!(f, "known-answer mismatch for {algorithm}: got {got}, want {want}") + } + Self::RoundtripVerify { algorithm, detail } => write!(f, "{algorithm} roundtrip shard failed verification: {detail}"), + Self::RoundtripReadback { algorithm } => write!(f, "{algorithm} roundtrip read back different bytes"), + Self::TamperNotRejected { algorithm, tampered } => { + write!(f, "{algorithm} tampered shard ({tampered}) was not rejected") + } + } + } +} + +impl std::error::Error for BitrotSelfTestError {} + +fn self_test_hex(bytes: &[u8]) -> String { + rustfs_utils::hex(bytes) +} + +// (kept as a named one-liner so every KAT failure site reads the same; the +// underlying formatter is the shared `rustfs_utils::hex`) + +/// Compare a digest against its pinned constant. Split out so a test can drive +/// it with a wrong constant and prove the mismatch path fires. +fn bitrot_kat_check( + algorithm: &'static str, + algo: &HashAlgorithm, + payload: &[u8], + expected: &[u8; 32], +) -> Result<(), BitrotSelfTestError> { + let digest = algo.hash_encode(payload); + let digest = digest.as_ref(); + if digest.len() != expected.len() || digest != expected.as_slice() { + return Err(BitrotSelfTestError::KnownAnswerMismatch { + algorithm, + got: self_test_hex(digest), + want: self_test_hex(expected), + }); + } + Ok(()) +} + +/// Encode `payload` with `shard_size` blocks, verify it end to end, and read +/// every block back through `BitrotReader` comparing bytes. +async fn bitrot_roundtrip_check( + algorithm: &'static str, + algo: HashAlgorithm, + payload: &[u8], + shard_size: usize, +) -> Result<(), BitrotSelfTestError> { + let mut writer = BitrotWriter::new(std::io::Cursor::new(Vec::::new()), shard_size, algo.clone()); + for chunk in payload.chunks(shard_size) { + writer + .write(chunk) + .await + .map_err(|err| BitrotSelfTestError::RoundtripVerify { + algorithm, + detail: format!("encode failed: {err}"), + })?; + } + let encoded = writer.into_inner().into_inner(); + + let on_disk = bitrot_shard_file_size(payload.len(), shard_size, algo.clone()); + if encoded.len() != on_disk { + return Err(BitrotSelfTestError::RoundtripVerify { + algorithm, + detail: format!("encoded {} bytes, size formula says {on_disk}", encoded.len()), + }); + } + bitrot_verify(std::io::Cursor::new(encoded.clone()), on_disk, payload.len(), algo.clone(), shard_size) + .await + .map_err(|err| BitrotSelfTestError::RoundtripVerify { + algorithm, + detail: err.to_string(), + })?; + + let mut reader = BitrotReader::new(std::io::Cursor::new(encoded), shard_size, algo, false); + let mut offset = 0usize; + while offset < payload.len() { + let want = shard_size.min(payload.len() - offset); + let mut buf = vec![0u8; want]; + let read = reader + .read(&mut buf) + .await + .map_err(|err| BitrotSelfTestError::RoundtripVerify { + algorithm, + detail: format!("read back failed at offset {offset}: {err}"), + })?; + if read != want || buf[..read] != payload[offset..offset + read] { + return Err(BitrotSelfTestError::RoundtripReadback { algorithm }); + } + offset += read; + } + Ok(()) +} + +/// Flip one byte and require `bitrot_verify` to reject the result. +async fn bitrot_tamper_check( + algorithm: &'static str, + algo: HashAlgorithm, + payload: &[u8], + shard_size: usize, + tampered: &'static str, + flip_at: usize, +) -> Result<(), BitrotSelfTestError> { + let mut writer = BitrotWriter::new(std::io::Cursor::new(Vec::::new()), shard_size, algo.clone()); + for chunk in payload.chunks(shard_size) { + writer.write(chunk).await.expect("self-test encode should not fail"); + } + let mut corrupt = writer.into_inner().into_inner(); + let flip_index = flip_at % corrupt.len(); + corrupt[flip_index] ^= 0x80; + + let on_disk = bitrot_shard_file_size(payload.len(), shard_size, algo.clone()); + match bitrot_verify(std::io::Cursor::new(corrupt), on_disk, payload.len(), algo, shard_size).await { + // The flipped byte must be rejected as a hash mismatch specifically, not + // by any incidental read error: an in-memory cursor cannot fail reads, + // so accepting any other failure here would mask a verify path that + // errors out before it ever compares hashes. + Err(err) if err.to_string().contains("hash mismatch") => Ok(()), + Ok(()) => Err(BitrotSelfTestError::TamperNotRejected { algorithm, tampered }), + Err(err) => Err(BitrotSelfTestError::RoundtripVerify { + algorithm, + detail: format!("tampered shard rejected with an unexpected error: {err}"), + }), + } +} + +/// Verify every bitrot algorithm this crate can write or verify in production: +/// both streaming Highway variants roundtrip end to end (encode → size formula +/// → `bitrot_verify` → read back) and reject a flipped byte in both the data +/// and the leading hash, while all three hashed algorithms reproduce their +/// pinned known-answer digests. +/// +/// Runs in well under a millisecond on 4 KiB of data; callers may run it inline +/// at startup. Pure CPU, no allocation beyond a few KiB of scratch. +pub async fn bitrot_self_test() -> Result<(), BitrotSelfTestError> { + let payload = bitrot_self_test_payload(); + + // Externally verifiable vector first: it guards the HashAlgorithm plumbing + // itself, before any self-pinned constants are consulted. + let abc = HashAlgorithm::SHA256.hash_encode(b"abc"); + if abc.as_ref() != BITROT_SELF_TEST_KAT_SHA256_ABC.as_slice() { + return Err(BitrotSelfTestError::KnownAnswerMismatch { + algorithm: "SHA256", + got: self_test_hex(abc.as_ref()), + want: self_test_hex(&BITROT_SELF_TEST_KAT_SHA256_ABC), + }); + } + + bitrot_kat_check( + "HighwayHash256S", + &HashAlgorithm::HighwayHash256S, + &payload, + &BITROT_SELF_TEST_KAT_HIGHWAY_HASH256S, + )?; + bitrot_kat_check( + "HighwayHash256SLegacy", + &HashAlgorithm::HighwayHash256SLegacy, + &payload, + &BITROT_SELF_TEST_KAT_HIGHWAY_HASH256S_LEGACY, + )?; + + for (algorithm, algo) in [ + ("HighwayHash256S", HashAlgorithm::HighwayHash256S), + ("HighwayHash256SLegacy", HashAlgorithm::HighwayHash256SLegacy), + ] { + // Full blocks plus a partial tail, exactly like a real part stripe. + let tail_len = 2 * 1024 + 333; + bitrot_roundtrip_check(algorithm, algo.clone(), &payload, 1024).await?; + bitrot_roundtrip_check(algorithm, algo.clone(), &payload[..tail_len], 1024).await?; + // One flipped byte in the final data block, one in the first leading + // hash: both must fail verification. + bitrot_tamper_check(algorithm, algo.clone(), &payload, 1024, "final data byte", payload.len() - 1).await?; + bitrot_tamper_check(algorithm, algo, &payload, 1024, "leading hash byte", 0).await?; + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::{ - BitrotReader, BitrotWriter, BitrotWriterWrapper, CustomWriter, bitrot_shard_file_size, bitrot_verify, write_all_vectored, + BitrotReader, BitrotWriter, BitrotWriterWrapper, CustomWriter, bitrot_kat_check, bitrot_self_test, + bitrot_self_test_payload, bitrot_shard_file_size, bitrot_verify, write_all_vectored, }; use super::{MAX_RETAINED_CHUNKS_PER_BLOCK, ShardChunkRead, ShardSource}; use bytes::Bytes; @@ -1090,6 +1343,32 @@ mod tests { } } + #[test] + fn bitrot_self_test_payload_is_deterministic() { + // Two independent builds of the payload must agree byte for byte, or + // the pinned known-answer digests below would be meaningless. + assert_eq!(bitrot_self_test_payload(), bitrot_self_test_payload()); + } + + #[test] + fn bitrot_self_test_rejects_a_wrong_known_answer_digest() { + let payload = bitrot_self_test_payload(); + let wrong = [0u8; 32]; + let err = bitrot_kat_check("HighwayHash256S", &HashAlgorithm::HighwayHash256S, &payload, &wrong) + .expect_err("a zeroed digest must never match"); + match err { + super::BitrotSelfTestError::KnownAnswerMismatch { algorithm, .. } => assert_eq!(algorithm, "HighwayHash256S"), + other => panic!("expected KnownAnswerMismatch, got {other:?}"), + } + } + + #[tokio::test] + async fn bitrot_self_test_passes() { + bitrot_self_test() + .await + .expect("the pinned digests and roundtrip checks must all pass on this platform"); + } + #[tokio::test] async fn vectored_test_writers_cover_fallback_flush_and_shutdown_paths() { let mut counting = VectoredCountingWriter::default(); @@ -1189,7 +1468,7 @@ mod tests { let last = corrupt.len() - 1; corrupt[last] ^= 0x80; let err = bitrot_verify( - Cursor::new(corrupt), + std::io::Cursor::new(corrupt), super::bitrot_shard_file_size(data.len(), shard_size, algo.clone()), data.len(), algo, @@ -1282,7 +1561,7 @@ mod tests { #[tokio::test] async fn bitrot_reader_rejects_output_buffers_larger_than_shard_size() { - let mut reader = BitrotReader::new(Cursor::new(Vec::::new()), 4, HashAlgorithm::None, false); + let mut reader = BitrotReader::new(std::io::Cursor::new(Vec::::new()), 4, HashAlgorithm::None, false); let mut out = [0u8; 5]; let err = reader .read(&mut out) @@ -1407,7 +1686,7 @@ mod tests { (HashAlgorithm::HighwayHash256, true), ] { let label = format!("{algo:?}"); - let writer = Cursor::new(Vec::::new()); + let writer = std::io::Cursor::new(Vec::::new()); let mut w = BitrotWriter::new(writer, shard_size, algo.clone()); w.write(&[7u8; 16]).await.unwrap(); let written = w.into_inner().into_inner(); @@ -1492,7 +1771,7 @@ mod tests { } async fn encode_one_block(payload: &[u8], shard_size: usize, algo: HashAlgorithm) -> Vec { - let mut w = BitrotWriter::new(Cursor::new(Vec::::new()), shard_size, algo); + let mut w = BitrotWriter::new(std::io::Cursor::new(Vec::::new()), shard_size, algo); w.write(payload).await.unwrap(); w.into_inner().into_inner() } @@ -1600,7 +1879,7 @@ mod tests { for algo in [HashAlgorithm::HighwayHash256S, HashAlgorithm::HighwayHash256SLegacy] { for &size in &[1usize, 16, 17, 32, 40, 48] { let payload: Vec = (0..size).map(|i| i as u8).collect(); - let mut w = BitrotWriter::new(Cursor::new(Vec::::new()), shard_size, algo.clone()); + let mut w = BitrotWriter::new(std::io::Cursor::new(Vec::::new()), shard_size, algo.clone()); for chunk in payload.chunks(shard_size) { w.write(chunk).await.unwrap(); } @@ -1674,14 +1953,14 @@ mod tests { w.write(&data).await.expect("write shard"); let mut via_read = vec![0u8; SHARD]; - let n1 = BitrotReader::new(Cursor::new(encoded.clone()), SHARD, algo.clone(), false) + let n1 = BitrotReader::new(std::io::Cursor::new(encoded.clone()), SHARD, algo.clone(), false) .read(&mut via_read) .await .expect("read"); // A buffer with only capacity — no initialized bytes at all. let mut via_append: Vec = Vec::with_capacity(SHARD); - let n2 = BitrotReader::new(Cursor::new(encoded), SHARD, algo.clone(), false) + let n2 = BitrotReader::new(std::io::Cursor::new(encoded), SHARD, algo.clone(), false) .read_appending(&mut via_append, SHARD) .await .expect("read_appending"); @@ -1706,7 +1985,7 @@ mod tests { encoded.truncate(encoded.len() - 1); let mut out: Vec = Vec::with_capacity(SHARD); - let err = BitrotReader::new(Cursor::new(encoded), SHARD, algo.clone(), false) + let err = BitrotReader::new(std::io::Cursor::new(encoded), SHARD, algo.clone(), false) .read_appending(&mut out, SHARD) .await .expect_err("a truncated shard must not succeed"); @@ -1732,7 +2011,7 @@ mod tests { encoded[last] ^= 0xff; let mut out: Vec = Vec::with_capacity(SHARD); - let err = BitrotReader::new(Cursor::new(encoded), SHARD, algo, false) + let err = BitrotReader::new(std::io::Cursor::new(encoded), SHARD, algo, false) .read_appending(&mut out, SHARD) .await .expect_err("a corrupt shard must not verify"); @@ -1844,7 +2123,7 @@ mod tests { "Cursor must be able to hand out a block, otherwise the fast path is dead code" ); assert_eq!(mem.position(), 8, "taking a block must advance like a read of the same length"); - let mut streamed = Cursor::new(encoded.clone()); + let mut streamed = std::io::Cursor::new(encoded.clone()); assert!( ShardSource::try_take_block(&mut streamed, 8).is_none(), "a non-Bytes source must stay on the streaming path" @@ -1872,7 +2151,7 @@ mod tests { ); let mut via_stream: Vec = Vec::with_capacity(SHARD); - BitrotReader::new(Cursor::new(encoded), SHARD, algo, false) + BitrotReader::new(std::io::Cursor::new(encoded), SHARD, algo, false) .read_appending(&mut via_stream, SHARD) .await .expect("streaming read"); diff --git a/rustfs/src/admin/handlers/system.rs b/rustfs/src/admin/handlers/system.rs index 96054ada5..c7ad93b54 100644 --- a/rustfs/src/admin/handlers/system.rs +++ b/rustfs/src/admin/handlers/system.rs @@ -417,6 +417,13 @@ struct SystemAdminDiscovery { struct ServerInfoResponse { info: InfoMessage, admin_discovery: SystemAdminDiscovery, + /// Startup bitrot algorithm self-test outcome (rustfs/backlog#1873): + /// `passed` (algorithms verified at boot), `failed` (a drifted hash + /// implementation — the process is serving with degraded integrity + /// checking unless `RUSTFS_BITROT_SELFTEST_STRICT` aborted it), or + /// `unknown` (not yet run or disabled). + #[serde(rename = "bitrotSelftest")] + bitrot_selftest: &'static str, } #[derive(Serialize)] @@ -433,6 +440,14 @@ fn system_admin_discovery(usecase: &DefaultAdminUsecase) -> SystemAdminDiscovery } } +fn bitrot_selftest_status_str() -> &'static str { + match crate::bitrot_selftest::bitrot_selftest_passed() { + Some(true) => "passed", + Some(false) => "failed", + None => "unknown", + } +} + #[async_trait::async_trait] impl Operation for ServerInfoHandler { async fn call(&self, req: S3Request, _params: Params<'_, '_>) -> S3Result> { @@ -464,6 +479,7 @@ impl Operation for ServerInfoHandler { let response = ServerInfoResponse { info, admin_discovery: system_admin_discovery(&usecase), + bitrot_selftest: bitrot_selftest_status_str(), }; let data = serde_json::to_vec(&response).map_err(|e| { @@ -1535,6 +1551,18 @@ mod tests { ); } + /// The startup bitrot self-test outcome must surface in server info as one + /// of three closed-set strings, never an internal enum or a null + /// (rustfs/backlog#1873). This test pins the string mapping; whether the + /// process-global cell holds Some(true)/Some(false)/None is owned by + /// `crate::bitrot_selftest`'s own tests. + #[test] + fn bitrot_selftest_status_str_is_a_closed_set_of_operators_strings() { + let rendered = super::bitrot_selftest_status_str(); + assert!(matches!(rendered, "passed" | "failed" | "unknown")); + assert_eq!(super::bitrot_selftest_status_str(), rendered); + } + #[test] fn server_info_response_exposes_admin_discovery_paths() { let usecase = DefaultAdminUsecase::without_context(); @@ -1556,6 +1584,7 @@ mod tests { pools: None, }, admin_discovery: system_admin_discovery(&usecase), + bitrot_selftest: super::bitrot_selftest_status_str(), }; let value = serde_json::to_value(response).expect("server info response should serialize"); diff --git a/rustfs/src/bitrot_selftest.rs b/rustfs/src/bitrot_selftest.rs new file mode 100644 index 000000000..4b8c1d1c4 --- /dev/null +++ b/rustfs/src/bitrot_selftest.rs @@ -0,0 +1,181 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Startup bitrot algorithm self-test (rustfs/backlog#1873). +//! +//! A drifted hash implementation fails silently in production: every shard +//! reads back "corrupt", heal rewrites healthy data, and cross-platform +//! clusters disagree about which copy is good. [`run_startup_bitrot_self_test`] +//! pins the algorithms once at process start — the check itself runs in well +//! under a millisecond on 4 KiB, so it executes inline before background +//! services come up and the result is published before the server accepts +//! traffic. +//! +//! Outcome surface: +//! - one structured `bitrot_selftest` log event (`passed`/`failed`/`skipped`), +//! - the `rustfs_bitrot_selftest_status` gauge (1=passed, 0=failed, 2=skipped), +//! - [`bitrot_selftest_passed`] for admin/health surfaces, +//! - `RUSTFS_BITROT_SELFTEST_STRICT=on` turns a failure into a startup error +//! (MinIO `bitrotSelfTest` Fatal parity); the default only degrades the +//! status so a bad build cannot brick an existing fleet on upgrade. + +use crate::storage_api::startup::background::{BitrotSelfTestError, bitrot_self_test}; +use metrics::gauge; +use std::future::Future; +use std::io; +use std::sync::atomic::{AtomicU8, Ordering}; +use std::time::Instant; +use tracing::{debug, error, info}; + +const LOG_COMPONENT_MAIN: &str = "main"; +const LOG_SUBSYSTEM_STARTUP: &str = "startup"; +const EVENT_BITROT_SELFTEST: &str = "bitrot_selftest"; +const METRIC_BITROT_SELFTEST_STATUS: &str = "rustfs_bitrot_selftest_status"; + +/// Gauge values for [`METRIC_BITROT_SELFTEST_STATUS`]. +const STATUS_PASSED: f64 = 1.0; +const STATUS_FAILED: f64 = 0.0; +const STATUS_SKIPPED: f64 = 2.0; + +/// Internal cell values for [`BITROT_SELF_TEST_STATUS`]. +const STATUS_CELL_UNSET: u8 = 0; +const STATUS_CELL_PASSED: u8 = 1; +const STATUS_CELL_FAILED: u8 = 2; + +static BITROT_SELF_TEST_STATUS: AtomicU8 = AtomicU8::new(STATUS_CELL_UNSET); + +/// Last recorded self-test outcome: `None` before the first run, then +/// `Some(true)` on a passing check and `Some(false)` on a failed one (a +/// skipped check never publishes, so it cannot read as a pass). The cell is +/// last-writer-wins rather than set-once: production runs the self-test once, +/// and last-writer-wins keeps tests that exercise both outcomes +/// order-independent. +pub fn bitrot_selftest_passed() -> Option { + match BITROT_SELF_TEST_STATUS.load(Ordering::Acquire) { + STATUS_CELL_UNSET => None, + STATUS_CELL_PASSED => Some(true), + STATUS_CELL_FAILED => Some(false), + _ => None, + } +} + +/// Run the bitrot self-test and publish the outcome. In strict mode a failure +/// is returned as an error so the caller aborts startup. +pub(crate) async fn run_startup_bitrot_self_test(enabled: bool, strict: bool) -> io::Result<()> { + run_startup_bitrot_self_test_with(enabled, strict, bitrot_self_test).await +} + +async fn run_startup_bitrot_self_test_with(enabled: bool, strict: bool, run_check: F) -> io::Result<()> +where + F: FnOnce() -> Fut, + Fut: Future>, +{ + if !enabled { + gauge!(METRIC_BITROT_SELFTEST_STATUS).set(STATUS_SKIPPED); + debug!( + target: "rustfs::main::run", + event = EVENT_BITROT_SELFTEST, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_STARTUP, + state = "skipped", + reason = "disabled", + "Bitrot self-test skipped" + ); + return Ok(()); + } + + let started = Instant::now(); + match run_check().await { + Ok(()) => { + BITROT_SELF_TEST_STATUS.store(STATUS_CELL_PASSED, Ordering::Release); + gauge!(METRIC_BITROT_SELFTEST_STATUS).set(STATUS_PASSED); + info!( + target: "rustfs::main::run", + event = EVENT_BITROT_SELFTEST, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_STARTUP, + state = "passed", + duration_us = started.elapsed().as_micros() as u64, + "Bitrot self-test passed" + ); + } + Err(err) => { + BITROT_SELF_TEST_STATUS.store(STATUS_CELL_FAILED, Ordering::Release); + gauge!(METRIC_BITROT_SELFTEST_STATUS).set(STATUS_FAILED); + error!( + target: "rustfs::main::run", + event = EVENT_BITROT_SELFTEST, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_STARTUP, + state = "failed", + duration_us = started.elapsed().as_micros() as u64, + error = %err, + "Bitrot self-test failed" + ); + if strict { + return Err(io::Error::other(format!("bitrot self-test failed: {err}"))); + } + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::{BITROT_SELF_TEST_STATUS, STATUS_CELL_UNSET, bitrot_selftest_passed, run_startup_bitrot_self_test_with}; + use crate::storage_api::startup::background::BitrotSelfTestError; + use std::future::ready; + use std::sync::atomic::Ordering; + + fn failing_check() -> impl Future> { + ready(Err(BitrotSelfTestError::RoundtripReadback { + algorithm: "HighwayHash256S", + })) + } + + /// All scenarios run sequentially inside one test: the status cell is + /// process-global, so parallel per-scenario tests would race the reset and + /// read each other's outcomes (the exact order-dependent flake class this + /// module exists to avoid). + #[tokio::test] + async fn startup_self_test_publishes_outcome_and_strict_gates_abort() { + BITROT_SELF_TEST_STATUS.store(STATUS_CELL_UNSET, Ordering::Release); + + // Skipped: publishes nothing, never fails, never aborts. + run_startup_bitrot_self_test_with(false, true, || async { Ok(()) }) + .await + .expect("a disabled self-test must not fail even in strict mode"); + assert_eq!(bitrot_selftest_passed(), None, "a skipped run must leave the status unset"); + + // Passing: publishes Some(true), never fails. + run_startup_bitrot_self_test_with(true, false, || async { Ok(()) }) + .await + .expect("a passing check must never fail startup"); + assert_eq!(bitrot_selftest_passed(), Some(true), "a passing run must publish Some(true)"); + + // Failing, non-strict: publishes Some(false) but startup continues. + run_startup_bitrot_self_test_with(true, false, failing_check) + .await + .expect("a failed check must not abort startup in non-strict mode"); + assert_eq!(bitrot_selftest_passed(), Some(false), "a failing run must publish Some(false)"); + + // Failing, strict: startup error carries the failure and the published + // outcome stays a failure. + let err = run_startup_bitrot_self_test_with(true, true, failing_check) + .await + .expect_err("strict mode must turn a failed check into a startup error"); + assert!(err.to_string().contains("bitrot self-test failed")); + assert_eq!(bitrot_selftest_passed(), Some(false)); + } +} diff --git a/rustfs/src/lib.rs b/rustfs/src/lib.rs index f8e9d893d..b60a2bd20 100644 --- a/rustfs/src/lib.rs +++ b/rustfs/src/lib.rs @@ -76,6 +76,7 @@ pub mod allocator_reclaim; pub mod app; pub mod auth; pub mod auth_keystone; +pub(crate) mod bitrot_selftest; pub mod capacity; pub mod cluster_snapshot; pub mod config; diff --git a/rustfs/src/module_switches.rs b/rustfs/src/module_switches.rs index fcb0ffcef..ced58b0ec 100644 --- a/rustfs/src/module_switches.rs +++ b/rustfs/src/module_switches.rs @@ -33,6 +33,8 @@ pub(crate) const ENV_SCANNER_ENABLED: &str = "RUSTFS_SCANNER_ENABLED"; pub(crate) const ENV_SCANNER_ENABLED_DEPRECATED: &str = "RUSTFS_ENABLE_SCANNER"; pub(crate) const ENV_HEAL_ENABLED: &str = "RUSTFS_HEAL_ENABLED"; pub(crate) const ENV_HEAL_ENABLED_DEPRECATED: &str = "RUSTFS_ENABLE_HEAL"; +pub(crate) const ENV_BITROT_SELFTEST_ENABLE: &str = "RUSTFS_BITROT_SELFTEST_ENABLE"; +pub(crate) const ENV_BITROT_SELFTEST_STRICT: &str = "RUSTFS_BITROT_SELFTEST_STRICT"; static AUDIT_MODULE_ENABLED: AtomicBool = AtomicBool::new(rustfs_config::DEFAULT_AUDIT_ENABLE); static NOTIFY_MODULE_ENABLED: AtomicBool = AtomicBool::new(rustfs_config::DEFAULT_NOTIFY_ENABLE); @@ -47,6 +49,18 @@ pub(crate) fn heal_enabled_from_env() -> bool { get_env_bool_with_aliases(ENV_HEAL_ENABLED, &[ENV_HEAL_ENABLED_DEPRECATED], true) } +/// Whether the startup bitrot algorithm self-test runs, defaulting to on +/// (rustfs/backlog#1873). +pub(crate) fn bitrot_selftest_enabled_from_env() -> bool { + rustfs_utils::get_env_bool(ENV_BITROT_SELFTEST_ENABLE, true) +} + +/// Whether a failed bitrot self-test aborts startup instead of only logging +/// and exposing a failed status, defaulting to off. +pub(crate) fn bitrot_selftest_strict_from_env() -> bool { + rustfs_utils::get_env_bool(ENV_BITROT_SELFTEST_STRICT, false) +} + /// Last published audit-module state. pub fn is_audit_module_enabled() -> bool { AUDIT_MODULE_ENABLED.load(Ordering::Relaxed) diff --git a/rustfs/src/startup_background.rs b/rustfs/src/startup_background.rs index 8cee2c502..fd5366740 100644 --- a/rustfs/src/startup_background.rs +++ b/rustfs/src/startup_background.rs @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::module_switches::{heal_enabled_from_env, scanner_enabled_from_env}; +use crate::bitrot_selftest::run_startup_bitrot_self_test; +use crate::module_switches::{ + bitrot_selftest_enabled_from_env, bitrot_selftest_strict_from_env, heal_enabled_from_env, scanner_enabled_from_env, +}; use crate::storage_api::startup::background::{ECStore, set_workload_admission_snapshot_provider}; use crate::workload_admission::RustFsWorkloadAdmissionSnapshotProvider; use rustfs_concurrency::WorkloadAdmissionSnapshotProvider; @@ -27,6 +30,12 @@ const LOG_SUBSYSTEM_STARTUP: &str = "startup"; const EVENT_BACKGROUND_SERVICES_CONFIGURED: &str = "background_services_configured"; pub(crate) async fn init_background_service_runtime(store: Arc) -> Result { + // Pin the bitrot algorithms before anything can write or verify a shard: + // the check costs well under a millisecond, and in strict mode a drifted + // build must abort here rather than after it has touched data + // (rustfs/backlog#1873). + run_startup_bitrot_self_test(bitrot_selftest_enabled_from_env(), bitrot_selftest_strict_from_env()).await?; + let _ = create_ahm_services_cancel_token(); let enable_scanner = scanner_enabled_from_env(); diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index 5d217f18e..d1e7c0f3a 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -569,6 +569,10 @@ pub(crate) mod ecstore_erasure { pub(crate) use rustfs_ecstore::api::erasure::{BitrotReader, Erasure}; } +/// Startup bitrot algorithm self-test (rustfs/backlog#1873), re-exported for +/// the root facade's background-startup section. +pub(crate) use rustfs_ecstore::api::erasure::{BitrotSelfTestError, bitrot_self_test}; + pub(crate) mod ecstore_storage { #[cfg(test)] pub(crate) use rustfs_ecstore::api::storage::init_local_disks; diff --git a/rustfs/src/storage_api.rs b/rustfs/src/storage_api.rs index b9b8d3c64..e90c1f528 100644 --- a/rustfs/src/storage_api.rs +++ b/rustfs/src/storage_api.rs @@ -214,7 +214,9 @@ pub(crate) mod startup { } pub(crate) mod background { - pub(crate) use crate::storage::storage_api::{ECStore, set_workload_admission_snapshot_provider}; + pub(crate) use crate::storage::storage_api::{ + BitrotSelfTestError, ECStore, bitrot_self_test, set_workload_admission_snapshot_provider, + }; } pub(crate) mod bucket_metadata { From 23b17c2d5ac9d6e4e7ebdb44ee4efb0bad102f5c Mon Sep 17 00:00:00 2001 From: houseme Date: Mon, 17 Aug 2026 15:04:49 +0800 Subject: [PATCH 07/10] feat(madmin): add a SigV4-signed admin client for heal and scanner APIs (HS-05) (#6166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit feat(madmin): add a SigV4-signed admin client for heal and scanner APIs The madmin crate held only wire types; automation and mc-style tooling had no way to drive the heal/scanner admin surface without hand-rolled HTTP. Add `AdminClient`, which signs with the same rustfs-signer path the server authenticates (UNSIGNED-PAYLOAD marker, matching RustFS peer admin calls) and wraps: - heal_start / heal_status / heal_stop over POST /rustfs/admin/v3/heal/ (bucket/prefix path params percent-encoded per segment; stop models the server's two cancel branches: token-scoped task status vs path-scoped start-success receipt); - background_heal_status, scanner_status (freshness typed), plus ilm_expiry_status / replacement_recovery_status passthroughs; - a public get_json escape hatch for endpoints not wrapped yet. Wire types follow the madmin-go model (SDK-owned mirrors pinned by round-trip tests): HealOpts with serde defaults so partial settings objects decode, HealScanMode accepting both the numeric and name encodings, and status structs that type the fields operators branch on while flattening unknown nested payloads verbatim so server additions cannot break the client. Errors map to a closed AdminClientError enum (InvalidEndpoint / Transport / HttpStatus with body / Decode). Tests cover wire round-trips, path building, both stop branches, error mapping, and — via a dependency-free raw-TCP test server — that signed requests carry a SigV4 Authorization header, the right method/path/ query, and the expected JSON body. Closes rustfs/backlog#1869 (first increment; single-sourcing the wire structs server-side and an embedded-server e2e roundtrip are noted as follow-ups there). Co-authored-by: heihutu --- Cargo.lock | 5 + crates/madmin/Cargo.toml | 5 + crates/madmin/src/client.rs | 851 ++++++++++++++++++++++++++++++++++++ crates/madmin/src/lib.rs | 2 + 4 files changed, 863 insertions(+) create mode 100644 crates/madmin/src/client.rs diff --git a/Cargo.lock b/Cargo.lock index 38637d173..b521a731d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9825,14 +9825,19 @@ name = "rustfs-madmin" version = "1.0.0-rc.2" dependencies = [ "hotpath", + "http 1.5.0", "humantime", "hyper", "jiff", + "reqwest", "rmp-serde", + "rustfs-signer", + "s3s", "serde", "serde_json", "sysinfo", "time", + "tokio", ] [[package]] diff --git a/crates/madmin/Cargo.toml b/crates/madmin/Cargo.toml index 6a1fe0ff5..dfd346334 100644 --- a/crates/madmin/Cargo.toml +++ b/crates/madmin/Cargo.toml @@ -37,7 +37,11 @@ hotpath-cpu = ["hotpath", "hotpath/hotpath-cpu"] [dependencies] hotpath.workspace = true humantime.workspace = true +http.workspace = true hyper = { workspace = true, features = ["http2", "http1", "server"] } +reqwest = { workspace = true, features = ["json"] } +rustfs-signer.workspace = true +s3s.workspace = true jiff = { workspace = true, features = ["serde"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["raw_value"] } @@ -49,3 +53,4 @@ doctest = false [dev-dependencies] rmp-serde.workspace = true +tokio = { workspace = true, features = ["macros", "rt-multi-thread", "net"] } diff --git a/crates/madmin/src/client.rs b/crates/madmin/src/client.rs new file mode 100644 index 000000000..273f922a3 --- /dev/null +++ b/crates/madmin/src/client.rs @@ -0,0 +1,851 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Admin API HTTP client for heal and scanner management (rustfs/backlog#1869). +//! +//! [`AdminClient`] speaks the `/rustfs/admin/v3` surface with S3 SigV4 +//! request signing (the same scheme the server's admin router authenticates), +//! so `mc`-style tooling and automation can drive heal start/query/cancel and +//! read background-heal / scanner status without hand-rolling HTTP. +//! +//! Wire structs in this module mirror the server-side shapes +//! (`rustfs/src/admin/handlers/heal.rs`, `handlers/scanner.rs`, +//! `rustfs-common/src/heal_channel.rs`), following the madmin-go model where +//! the SDK owns its own copies and round-trip tests pin the encoding. Deeply +//! nested status payloads that the server composes from runtime types are +//! carried through as `serde_json::Value` and flattened maps rather than +//! duplicated field-for-field, so the client cannot silently drift on fields +//! it never interprets. + +use crate::heal_commands::HealResultItem; +use http::Method; +use serde::{Deserialize, Serialize, de}; +use std::time::Duration; + +/// Default admin API path prefix on a RustFS endpoint. +pub const DEFAULT_ADMIN_API_PREFIX: &str = "/rustfs/admin"; +/// Default SigV4 region when the server has no explicit region configured. +pub const DEFAULT_REGION: &str = "us-east-1"; + +/// Scan mode for a heal request, mirroring the server's numeric-or-name wire +/// encoding (`0` unknown/default, `1` normal, `2` deep). +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum HealScanMode { + /// Server default; behaves as [`HealScanMode::Normal`]. + #[default] + Unknown, + /// Metadata-level checks only. + Normal, + /// Full bitrot verification while healing. + Deep, +} + +impl HealScanMode { + fn wire_number(self) -> u8 { + match self { + Self::Unknown => 0, + Self::Normal => 1, + Self::Deep => 2, + } + } + + fn from_wire_number(value: u8) -> Option { + match value { + 0 => Some(Self::Unknown), + 1 => Some(Self::Normal), + 2 => Some(Self::Deep), + _ => None, + } + } + + fn from_wire_name(value: &str) -> Option { + match value { + "unknown" => Some(Self::Unknown), + "normal" => Some(Self::Normal), + "deep" => Some(Self::Deep), + _ => None, + } + } +} + +impl Serialize for HealScanMode { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_u8(self.wire_number()) + } +} + +impl<'de> Deserialize<'de> for HealScanMode { + fn deserialize>(deserializer: D) -> Result { + struct HealScanModeVisitor; + + impl de::Visitor<'_> for HealScanModeVisitor { + type Value = HealScanMode; + + fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str("a heal scan mode number or name") + } + + fn visit_u64(self, value: u64) -> Result { + u8::try_from(value) + .ok() + .and_then(HealScanMode::from_wire_number) + .ok_or_else(|| E::custom(format!("unknown heal scan mode number: {value}"))) + } + + fn visit_str(self, value: &str) -> Result { + HealScanMode::from_wire_name(value).ok_or_else(|| E::custom(format!("unknown heal scan mode name: {value}"))) + } + } + + deserializer.deserialize_any(HealScanModeVisitor) + } +} + +/// Heal options for an admin heal request (mirror of the server body type). +/// Fields default on decode: a client should tolerate a server response whose +/// settings object omits fields it never set. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct HealOpts { + #[serde(default)] + pub recursive: bool, + #[serde(rename = "dryRun", default)] + pub dry_run: bool, + #[serde(default)] + pub remove: bool, + #[serde(default)] + pub recreate: bool, + #[serde(rename = "scanMode", default)] + pub scan_mode: HealScanMode, + #[serde(rename = "updateParity", default)] + pub update_parity: bool, + #[serde(rename = "nolock", default)] + pub no_lock: bool, + #[serde(rename = "pool", default)] + pub pool: Option, + #[serde(rename = "set", default)] + pub set: Option, +} + +/// Successful heal start / path-scoped cancel response. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct HealStartSuccess { + pub client_token: String, + pub client_address: String, + #[serde(default)] + pub start_time: String, +} + +/// Heal task status response (query, cancel-with-token, start-then-poll). +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct HealTaskStatus { + /// `running` | `finished` | `stopped` | `notFound`. + pub summary: String, + /// Failure detail for stopped tasks; empty otherwise. + #[serde(rename = "detail", default)] + pub failure_detail: String, + #[serde(default)] + pub start_time: String, + #[serde(default)] + pub settings: HealOpts, + #[serde(default)] + pub items: Vec, + #[serde(default)] + pub truncated: bool, + /// Live progress snapshot; the exact shape is owned by the heal runtime. + #[serde(default)] + pub progress: Option, +} + +/// `POST /v3/background-heal/status` response. Known top-level fields are +/// typed; the flattened heal info and operations matrix pass through verbatim. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct BackgroundHealStatus { + /// `disabled` | `uninitialized` | `idle` | `active` | `degraded`. + pub state: String, + #[serde(default)] + pub heal_queue_length: u64, + #[serde(default)] + pub heal_active_tasks: u64, + #[serde(default)] + pub cluster_status_complete: bool, + #[serde(default)] + pub progress: Option, + /// Remaining wire fields (flattened `BackgroundHealInfo` plus the + /// priority-by-source operations matrix), carried verbatim. + #[serde(flatten)] + pub extra: serde_json::Map, +} + +/// `GET /v3/scanner/status` response, typed at the fields operators branch +/// on; everything else passes through verbatim. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ScannerStatus { + pub enabled: bool, + /// `fresh` | `stale` | `unknown`; absent when the scanner never completed + /// a cycle. + #[serde(default)] + pub freshness: Option, + #[serde(flatten)] + pub extra: serde_json::Map, +} + +/// Freshness block of the scanner status response. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ScannerFreshness { + /// `fresh` | `stale` | `unknown`. + pub state: String, +} + +impl ScannerStatus { + /// Convenience accessor for the freshness state string. + pub fn freshness(&self) -> &str { + self.freshness + .as_ref() + .map(|freshness| freshness.state.as_str()) + .unwrap_or("unknown") + } +} + +/// Everything that can go wrong in an admin client call. +#[derive(Debug)] +pub enum AdminClientError { + /// The endpoint URL could not be parsed. + InvalidEndpoint(String), + /// Request build/send failed (DNS, connect, timeout, body read). + Transport(reqwest::Error), + /// The server answered a non-2xx status. + HttpStatus { status: u16, body: String }, + /// The response body did not decode into the expected shape. + Decode { message: String }, +} + +impl std::fmt::Display for AdminClientError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::InvalidEndpoint(message) => write!(f, "invalid admin endpoint: {message}"), + Self::Transport(err) => write!(f, "admin request transport failure: {err}"), + Self::HttpStatus { status, body } => write!(f, "admin request failed with HTTP {status}: {body}"), + Self::Decode { message } => write!(f, "admin response decode failure: {message}"), + } + } +} + +impl std::error::Error for AdminClientError {} + +impl From for AdminClientError { + fn from(err: reqwest::Error) -> Self { + Self::Transport(err) + } +} + +/// A signed client for a RustFS admin API. +#[derive(Debug, Clone)] +pub struct AdminClient { + endpoint: reqwest::Url, + access_key: String, + secret_key: String, + session_token: String, + region: String, + api_prefix: String, + http: reqwest::Client, +} + +impl AdminClient { + /// Build a client for `endpoint` (e.g. `http://127.0.0.1:9000`) using root + /// or admin credentials. Requests are SigV4-signed with the same scheme + /// the server's admin router authenticates. + pub fn new(endpoint: &str, access_key: &str, secret_key: &str) -> Result { + let url = reqwest::Url::parse(endpoint).map_err(|err| AdminClientError::InvalidEndpoint(err.to_string()))?; + if url.host_str().is_none() { + return Err(AdminClientError::InvalidEndpoint("endpoint has no host".to_string())); + } + let http = reqwest::Client::builder() + .connect_timeout(Duration::from_secs(10)) + .timeout(Duration::from_secs(30)) + .build() + .map_err(AdminClientError::Transport)?; + Ok(Self { + endpoint: url, + access_key: access_key.to_string(), + secret_key: secret_key.to_string(), + session_token: String::new(), + region: DEFAULT_REGION.to_string(), + api_prefix: DEFAULT_ADMIN_API_PREFIX.to_string(), + http, + }) + } + + /// Attach an STS session token (signed as `x-amz-security-token`). + pub fn with_session_token(mut self, session_token: impl Into) -> Self { + self.session_token = session_token.into(); + self + } + + /// Override the SigV4 region (defaults to `us-east-1`, matching a + /// region-less RustFS deployment). + pub fn with_region(mut self, region: impl Into) -> Self { + self.region = region.into(); + self + } + + /// Override the admin API path prefix (defaults to `/rustfs/admin`). + pub fn with_api_prefix(mut self, prefix: impl Into) -> Self { + self.api_prefix = prefix.into(); + self + } + + /// Start a heal. `bucket` empty and `prefix` empty heals the whole + /// deployment (requires `recursive` or a `pool`/`set` pair in `opts`, + /// enforced server-side); a bucket alone heals the bucket (the server + /// forces `recursive` for bucket heals). + pub async fn heal_start( + &self, + bucket: Option<&str>, + prefix: Option<&str>, + opts: &HealOpts, + force_start: bool, + ) -> Result { + let body = serde_json::to_vec(opts).map_err(|err| AdminClientError::Decode { + message: err.to_string(), + })?; + let mut query = Vec::new(); + if force_start { + query.push(("forceStart", "true".to_string())); + } + self.post_json(&heal_path(bucket, prefix), &query, body).await + } + + /// Query the status of the heal identified by `client_token` (the token + /// returned by [`Self::heal_start`]) at the path it was started on. + pub async fn heal_status( + &self, + bucket: Option<&str>, + prefix: Option<&str>, + client_token: &str, + ) -> Result { + self.post_json(&heal_path(bucket, prefix), &[("clientToken", client_token.to_string())], Vec::new()) + .await + } + + /// Stop a heal: with a `client_token` only that task is cancelled and its + /// final status returned; without one, every heal task at the path is + /// cancelled (the server answers with a start-success-shaped receipt). + pub async fn heal_stop( + &self, + bucket: Option<&str>, + prefix: Option<&str>, + client_token: Option<&str>, + ) -> Result { + let mut query = vec![("forceStop", "true".to_string())]; + if let Some(token) = client_token { + query.push(("clientToken", token.to_string())); + } + match client_token { + Some(_) => { + let status: HealTaskStatus = self.post_json(&heal_path(bucket, prefix), &query, Vec::new()).await?; + Ok(HealStopOutcome::Stopped(status)) + } + None => { + let success: HealStartSuccess = self.post_json(&heal_path(bucket, prefix), &query, Vec::new()).await?; + Ok(HealStopOutcome::PathStopped(success)) + } + } + } + + /// Cluster-aggregated background heal status. + pub async fn background_heal_status(&self) -> Result { + self.get_json("/v3/background-heal/status").await + } + + /// Data scanner status (enabled state, freshness, runtime config). + pub async fn scanner_status(&self) -> Result { + self.get_json("/v3/scanner/status").await + } + + /// ILM expiry worker status. The payload is owned by the expiry + /// subsystem and still evolving; returned verbatim. + pub async fn ilm_expiry_status(&self) -> Result { + self.get_json("/v3/ilm/expiry/status").await + } + + /// Durable replacement-recovery status (admin v4). The payload is owned + /// by the heal runtime; returned verbatim. + pub async fn replacement_recovery_status(&self) -> Result { + self.get_json("/v4/heal/replacement-recovery").await + } + + /// Signed GET returning a decoded JSON body; escape hatch for endpoints + /// this client does not wrap yet. + pub async fn get_json Deserialize<'de>>(&self, path: &str) -> Result { + let url = self.url_for(path, &[])?; + let request = self.sign_and_build(Method::GET, url, Vec::new(), None).await?; + self.execute(request).await + } + + /// Signed POST returning a decoded JSON body. + async fn post_json Deserialize<'de>>( + &self, + path: &str, + query: &[(&str, String)], + body: Vec, + ) -> Result { + let content_type = if body.is_empty() { None } else { Some("application/json") }; + let url = self.url_for(path, query)?; + let request = self.sign_and_build(Method::POST, url, body, content_type).await?; + self.execute(request).await + } + + fn url_for(&self, path: &str, query: &[(&str, String)]) -> Result { + let mut url = self + .endpoint + .join(&format!("{}{}", self.api_prefix.trim_end_matches('/'), path)) + .map_err(|err| AdminClientError::InvalidEndpoint(err.to_string()))?; + if !query.is_empty() { + let mut pairs = url.query_pairs_mut(); + for (key, value) in query { + pairs.append_pair(key, value); + } + } + Ok(url) + } + + /// Build a SigV4-signed request via the same signer the server trusts, + /// then hand the signed headers to the HTTP client. The signature covers + /// method, path, query, and an unsigned-payload marker — the same shape + /// RustFS itself sends for peer admin calls. + async fn sign_and_build( + &self, + method: Method, + url: reqwest::Url, + body: Vec, + content_type: Option<&str>, + ) -> Result { + let authority = match (url.host_str(), url.port_or_known_default()) { + (Some(host), Some(port)) => format!("{host}:{port}"), + _ => return Err(AdminClientError::InvalidEndpoint("endpoint has no authority".to_string())), + }; + let mut builder = http::Request::builder() + .method(method.clone()) + .uri(url.as_str()) + .header(http::header::HOST, &authority) + .header("x-amz-content-sha256", rustfs_signer::constants::UNSIGNED_PAYLOAD); + if let Some(content_type) = content_type { + builder = builder.header(http::header::CONTENT_TYPE, content_type); + } + let unsigned = builder + .body(s3s::Body::empty()) + .map_err(|err| AdminClientError::InvalidEndpoint(format!("build request failed: {err}")))?; + let signed = rustfs_signer::sign_v4( + unsigned, + body.len() as i64, + &self.access_key, + &self.secret_key, + &self.session_token, + &self.region, + ); + + let mut request = self + .http + .request(method, url) + .body(body) + .build() + .map_err(AdminClientError::Transport)?; + let headers = request.headers_mut(); + for (name, value) in signed.headers().iter() { + // HOST is owned by the HTTP client; the signed value above was + // built from the same URL authority, so they always agree. + if name == http::header::HOST { + continue; + } + headers.insert(name, value.clone()); + } + Ok(request) + } + + async fn execute Deserialize<'de>>(&self, request: reqwest::Request) -> Result { + let response = self.http.execute(request).await?; + let status = response.status(); + let bytes = response.bytes().await?; + if !status.is_success() { + return Err(AdminClientError::HttpStatus { + status: status.as_u16(), + body: String::from_utf8_lossy(&bytes).into_owned(), + }); + } + serde_json::from_slice(&bytes).map_err(|err| AdminClientError::Decode { + message: err.to_string(), + }) + } +} + +/// Response of [`AdminClient::heal_stop`]: cancelling a single tokened task +/// answers with that task's status, cancelling a whole path answers with a +/// start-success-shaped receipt. +#[derive(Debug, Clone)] +pub enum HealStopOutcome { + Stopped(HealTaskStatus), + PathStopped(HealStartSuccess), +} + +fn heal_path(bucket: Option<&str>, prefix: Option<&str>) -> String { + match (bucket, prefix) { + (Some(bucket), Some(prefix)) if !bucket.is_empty() && !prefix.is_empty() => { + format!("/v3/heal/{}/{}", percent_encode_path_segment(bucket), percent_encode_path_segment(prefix)) + } + (Some(bucket), Some(_)) | (Some(bucket), None) if !bucket.is_empty() => { + format!("/v3/heal/{}", percent_encode_path_segment(bucket)) + } + _ => "/v3/heal/".to_string(), + } +} + +/// Encode a single path segment (slashes are content, not separators, inside +/// bucket/prefix path params). +fn percent_encode_path_segment(segment: &str) -> String { + let mut out = String::with_capacity(segment.len()); + for byte in segment.bytes() { + match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => out.push(byte as char), + _ => out.push_str(&format!("%{byte:02X}")), + } + } + out +} + +#[cfg(test)] +mod tests { + use super::{ + AdminClient, AdminClientError, BackgroundHealStatus, HealOpts, HealScanMode, HealStartSuccess, HealTaskStatus, + ScannerStatus, heal_path, percent_encode_path_segment, + }; + use serde_json::json; + use std::sync::{Arc, Mutex}; + + #[test] + fn heal_paths_cover_root_bucket_and_prefix() { + assert_eq!(heal_path(None, None), "/v3/heal/"); + assert_eq!(heal_path(Some(""), Some("")), "/v3/heal/"); + assert_eq!(heal_path(Some("bucket"), None), "/v3/heal/bucket"); + assert_eq!(heal_path(Some("bucket"), Some("pre/fix")), "/v3/heal/bucket/pre%2Ffix"); + } + + #[test] + fn path_segments_percent_encode_reserved_characters() { + assert_eq!(percent_encode_path_segment("a b"), "a%20b"); + assert_eq!(percent_encode_path_segment("a/b"), "a%2Fb"); + assert_eq!(percent_encode_path_segment("ü"), "%C3%BC"); + } + + #[test] + fn heal_opts_round_trip_through_the_server_wire_shape() { + let opts = HealOpts { + recursive: true, + dry_run: false, + remove: true, + recreate: false, + scan_mode: HealScanMode::Deep, + update_parity: true, + no_lock: false, + pool: Some(1), + set: Some(2), + }; + let wire = serde_json::to_value(&opts).unwrap(); + assert_eq!(wire["scanMode"], json!(2), "the server body decodes scanMode as a number"); + let back: HealOpts = serde_json::from_value(wire).unwrap(); + assert_eq!(back.scan_mode, HealScanMode::Deep); + assert_eq!(back.pool, Some(1)); + } + + #[test] + fn heal_scan_mode_accepts_both_wire_encodings() { + assert_eq!(serde_json::from_value::(json!(1)).unwrap(), HealScanMode::Normal); + assert_eq!(serde_json::from_value::(json!("deep")).unwrap(), HealScanMode::Deep); + assert!(serde_json::from_value::(json!(9)).is_err()); + assert!(serde_json::from_value::(json!("sideways")).is_err()); + } + + #[test] + fn heal_task_status_decodes_the_server_response_shape() { + let raw = json!({ + "summary": "finished", + "detail": "", + "startTime": "2026-08-17T00:00:00Z", + "settings": {"recursive": false, "scanMode": 1}, + "items": [{ + "resultId": 1, "type": "object", "bucket": "b", "object": "o", "versionId": "", "detail": "", + "parityBlocks": 2, "dataBlocks": 2, "diskCount": 4, "setCount": 1, + "before": {"drives": []}, "after": {"drives": []}, "objectSize": 128 + }], + "truncated": false + }); + let status: HealTaskStatus = serde_json::from_value(raw).unwrap(); + assert_eq!(status.summary, "finished"); + assert_eq!(status.items.len(), 1); + assert_eq!(status.settings.scan_mode, HealScanMode::Normal); + assert!(status.progress.is_none()); + } + + #[test] + fn background_heal_status_types_known_fields_and_passes_the_rest_through() { + let raw = json!({ + "state": "active", + "bitrotStartTime": "t", + "healQueueLength": 3, + "healActiveTasks": 1, + "healOperations": {"queueLength": 3}, + "clusterStatusComplete": true + }); + let status: BackgroundHealStatus = serde_json::from_value(raw).unwrap(); + assert_eq!(status.state, "active"); + assert_eq!(status.heal_queue_length, 3); + assert!(status.cluster_status_complete); + assert!(status.extra.contains_key("healOperations"), "unknown nested payloads must pass through"); + } + + #[test] + fn scanner_status_defaults_freshness_to_unknown() { + let raw = json!({"enabled": true, "freshness": {"state": "stale"}, "metrics": {}}); + let status: ScannerStatus = serde_json::from_value(raw).unwrap(); + assert_eq!(status.freshness(), "stale"); + let bare: ScannerStatus = serde_json::from_value(json!({"enabled": false})).unwrap(); + assert_eq!(bare.freshness(), "unknown"); + } + + #[test] + fn invalid_endpoint_is_rejected_without_io() { + let err = AdminClient::new("not a url", "ak", "sk").unwrap_err(); + assert!(matches!(err, AdminClientError::InvalidEndpoint(_))); + } + + #[tokio::test] + async fn signed_requests_carry_sigv4_authorization_and_correct_target() { + let server = TestServer::spawn(r#"{"clientToken":"token-1","clientAddress":"127.0.0.1:9","startTime":"t"}"#, 200).await; + let client = AdminClient::new(&format!("http://{}", server.addr), "minioadmin", "minioadmin") + .expect("client builds against the test server"); + + let start: HealStartSuccess = client + .heal_start( + Some("bucket"), + None, + &HealOpts { + recursive: true, + ..Default::default() + }, + false, + ) + .await + .expect("signed heal start decodes"); + + assert_eq!(start.client_token, "token-1"); + let request = server.recorded(); + assert_eq!(request.method, "POST"); + assert_eq!(request.path, "/rustfs/admin/v3/heal/bucket"); + assert!(!request.query.contains("forceStart"), "absent flags must not be sent"); + let auth = request.header("authorization").expect("request must be signed"); + assert!(auth.starts_with("AWS4-HMAC-SHA256"), "SigV4 scheme, got: {auth}"); + assert!(auth.contains("Credential=minioadmin/"), "credentials must be in the Authorization header"); + assert_eq!( + request.header("x-amz-content-sha256").as_deref(), + Some("UNSIGNED-PAYLOAD"), + "the client signs the same payload marker RustFS peer calls use" + ); + assert_eq!(request.header("content-type").as_deref(), Some("application/json")); + assert!(request.body.contains("\"recursive\":true")); + } + + #[tokio::test] + async fn query_sends_client_token_on_the_same_path() { + let body = r#"{"summary":"running","detail":"","settings":{"recursive":false},"items":[],"truncated":false}"#; + let server = TestServer::spawn(body, 200).await; + let client = AdminClient::new(&format!("http://{}", server.addr), "ak", "sk").unwrap(); + + let status = client + .heal_status(Some("bucket"), None, "token-1") + .await + .expect("status decodes"); + assert_eq!(status.summary, "running"); + let request = server.recorded(); + assert_eq!(request.path, "/rustfs/admin/v3/heal/bucket"); + assert!(request.query.contains("clientToken=token-1")); + assert!(!request.query.contains("forceStop")); + } + + #[tokio::test] + async fn stop_without_token_takes_the_path_cancel_branch() { + let server = TestServer::spawn(r#"{"clientToken":"path","clientAddress":"c","startTime":"t"}"#, 200).await; + let client = AdminClient::new(&format!("http://{}", server.addr), "ak", "sk").unwrap(); + + let outcome = client.heal_stop(Some("bucket"), None, None).await.expect("path stop decodes"); + assert!(matches!(outcome, super::HealStopOutcome::PathStopped(_))); + let request = server.recorded(); + assert!(request.query.contains("forceStop=true")); + assert!(!request.query.contains("clientToken")); + } + + #[tokio::test] + async fn http_error_status_maps_to_a_typed_error_with_body() { + let server = TestServer::spawn(r#"{"code":"AccessDenied","message":"denied"}"#, 403).await; + let client = AdminClient::new(&format!("http://{}", server.addr), "ak", "sk").unwrap(); + let err = client.scanner_status().await.unwrap_err(); + match err { + AdminClientError::HttpStatus { status, body } => { + assert_eq!(status, 403); + assert!(body.contains("AccessDenied")); + } + other => panic!("expected HttpStatus, got {other:?}"), + } + } + + #[tokio::test] + async fn malformed_success_body_maps_to_a_decode_error() { + let server = TestServer::spawn("not json", 200).await; + let client = AdminClient::new(&format!("http://{}", server.addr), "ak", "sk").unwrap(); + assert!(matches!(client.scanner_status().await.unwrap_err(), AdminClientError::Decode { .. })); + } + + /// One recorded request, parsed off the wire with the minimum needed for + /// assertions: method, path, query, headers, body. + #[derive(Debug, Clone)] + struct RecordedRequest { + method: String, + path: String, + query: String, + headers: Vec<(String, String)>, + body: String, + } + + impl RecordedRequest { + fn header(&self, name: &str) -> Option { + self.headers + .iter() + .find(|(key, _)| key.eq_ignore_ascii_case(name)) + .map(|(_, value)| value.clone()) + } + } + + /// Minimal HTTP/1.1 server: one canned response per connection, every + /// request recorded behind an `Arc`. Deliberately dependency-free — + /// the assertions only need the raw request bytes. + struct TestServer { + addr: std::net::SocketAddr, + requests: Arc>>, + } + + impl TestServer { + async fn spawn(response_body: &'static str, status: u16) -> Self { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind ephemeral port"); + let addr = listener.local_addr().expect("local addr"); + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + + let recorded = requests.clone(); + tokio::spawn(async move { + let reason = if status == 200 { "OK" } else { "Forbidden" }; + let response = format!( + "HTTP/1.1 {status} {reason}\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{response_body}", + response_body.len() + ); + // Each request is a fresh connection (connection: close); a + // bounded loop serves every call a test makes while letting + // the task exit instead of lingering for the whole process. + for _ in 0..16 { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let mut buffer = Vec::with_capacity(2048); + let mut chunk = [0u8; 2048]; + // Read headers plus content-length body, or stop on close. + loop { + if let Some(end) = find_header_end(&buffer) { + let content_length = extract_content_length(&buffer[..end]); + if buffer.len() >= end + content_length { + break; + } + } + let n = match stream.read(&mut chunk).await { + Ok(0) | Err(_) => break, + Ok(n) => n, + }; + buffer.extend_from_slice(&chunk[..n]); + if buffer.len() > 64 * 1024 { + break; + } + } + if let Some(request) = parse_request(&buffer) { + recorded.lock().expect("recorded lock").push(request); + } + let _ = stream.write_all(response.as_bytes()).await; + let _ = stream.shutdown().await; + } + }); + + Self { addr, requests } + } + + fn recorded(&self) -> RecordedRequest { + self.requests + .lock() + .expect("recorded lock") + .last() + .cloned() + .expect("the client call must have produced one recorded request") + } + } + + fn find_header_end(buffer: &[u8]) -> Option { + buffer.windows(4).position(|window| window == b"\r\n\r\n").map(|pos| pos + 4) + } + + fn extract_content_length(headers: &[u8]) -> usize { + let text = String::from_utf8_lossy(headers).to_ascii_lowercase(); + text.lines() + .find_map(|line| line.strip_prefix("content-length:")) + .and_then(|value| value.trim().parse().ok()) + .unwrap_or(0) + } + + fn parse_request(raw: &[u8]) -> Option { + let end = find_header_end(raw)?; + let head = String::from_utf8_lossy(&raw[..end]); + let body = String::from_utf8_lossy(&raw[end..]).into_owned(); + let mut lines = head.lines(); + let request_line = lines.next()?; + let mut parts = request_line.split_whitespace(); + let method = parts.next()?.to_string(); + let target = parts.next()?.to_string(); + let (path, query) = match target.split_once('?') { + Some((path, query)) => (path.to_string(), query.to_string()), + None => (target, String::new()), + }; + let headers = lines + .filter_map(|line| line.split_once(':')) + .map(|(name, value)| (name.trim().to_string(), value.trim().to_string())) + .collect(); + Some(RecordedRequest { + method, + path, + query, + headers, + body, + }) + } +} diff --git a/crates/madmin/src/lib.rs b/crates/madmin/src/lib.rs index 154663b11..a9d4bd8b9 100644 --- a/crates/madmin/src/lib.rs +++ b/crates/madmin/src/lib.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +pub mod client; pub mod group; pub mod heal_commands; pub mod health; @@ -25,6 +26,7 @@ pub mod trace; pub mod user; pub mod utils; +pub use client::*; pub use group::*; pub use info_commands::*; pub use policy::*; From 984c7057132936a4772d29c1e209903cca3db85a Mon Sep 17 00:00:00 2001 From: houseme Date: Mon, 17 Aug 2026 16:24:54 +0800 Subject: [PATCH 08/10] docs(ecstore): fix bitrot comment typo (#6168) Co-authored-by: heihutu --- _typos.toml | 1 + crates/ecstore/src/erasure/coding/bitrot.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/_typos.toml b/_typos.toml index 12d50d60e..7ff843137 100644 --- a/_typos.toml +++ b/_typos.toml @@ -40,6 +40,7 @@ mak = "mak" gae = "gae" GAE = "GAE" thr = "thr" +mis = "mis" # s3-tests original test names (cannot be changed) nonexisted = "nonexisted" consts = "consts" diff --git a/crates/ecstore/src/erasure/coding/bitrot.rs b/crates/ecstore/src/erasure/coding/bitrot.rs index 698b1b02c..947bbb68e 100644 --- a/crates/ecstore/src/erasure/coding/bitrot.rs +++ b/crates/ecstore/src/erasure/coding/bitrot.rs @@ -835,7 +835,7 @@ pub const BITROT_SELF_TEST_PAYLOAD_LEN: usize = 4096; /// Known-answer digest of [`bitrot_self_test_payload`] under `HighwayHash256S` /// (the production default). Pinned so any platform or build where the -/// implementation drifts fails startup instead of mis-hashing shards. +/// implementation drifts fails startup instead of miss-hashing shards. const BITROT_SELF_TEST_KAT_HIGHWAY_HASH256S: [u8; 32] = [ 0xb9, 0x32, 0xa2, 0xaa, 0x4a, 0xb7, 0x33, 0x6a, 0xa3, 0xca, 0x7e, 0x61, 0x9d, 0x86, 0x52, 0x14, 0x6e, 0x7f, 0xd8, 0x9e, 0xea, 0x08, 0xd9, 0x8c, 0x33, 0x85, 0x87, 0x19, 0x30, 0xd6, 0xed, 0x06, From e0b87b0e7e48d5db66358dde292a80498df6e6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Mon, 17 Aug 2026 17:47:36 +0800 Subject: [PATCH 09/10] fix(site-replication): admit only verifiable peer-edit fences (#6123) --- rustfs/src/admin/handlers/site_replication.rs | 219 +++++++++++++++++- 1 file changed, 214 insertions(+), 5 deletions(-) diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index fba311ba9..18eef001b 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -6023,7 +6023,10 @@ fn edit_generation_wall_clock() -> u64 { /// node's clock behind the clock that fed the previous lifetime) mints /// below the stale mark and the origin stays fenced — but only until real /// time passes the previous lifetime's last allocation, because every later -/// allocation takes the wall-clock floor again. Bounded by the skew, +/// allocation takes the wall-clock floor again (and never longer than +/// [`PEER_EDIT_FENCE_STALENESS_WINDOW_NANOS`]: a regression past the window +/// leaves the mark implausibly distant and the origin runs unfenced +/// immediately). Bounded by the skew, /// self-healing, and no rollback window beyond the plain counter's: a /// delivery applies only at or above the receiver's mark, so the one /// cross-lifetime interleaving that can apply stale content — a @@ -6063,6 +6066,52 @@ fn peer_edit_fence(queries: &HashMap) -> Option<(String, u64)> { Some((origin.clone(), generation)) } +/// How far below the recorded high-water mark a delivery may sit and still +/// be fenced as stale. The distance a GENUINE superseded delivery can trail +/// its origin's mark is small: retransmissions re-run the sender flow and +/// mint a fresh generation (the retry queue keys on the bare path and never +/// replays a fenced URL), so only an in-flight straggler of the losing +/// fan-out race trails the mark, by delivery latency — minutes at the +/// outside. A mark further above than this window cannot be explained by +/// any genuine race, only by a forged fence (the shared service account +/// lets any peer stamp any origin) or by a persisted clock excursion the +/// origin has since left behind — and fencing on it would silently drop the +/// origin's real edits, so the stale check ignores it instead. +const PEER_EDIT_FENCE_STALENESS_WINDOW_NANOS: u64 = 24 * 60 * 60 * 1_000_000_000; + +/// Whether an incoming fence may be honoured, as far as this site can vouch +/// for it. The sender's identity is unverifiable (shared service account), +/// so the check runs over what the receiving state knows: the claimed origin +/// must be a site this state currently replicates with — the same membership +/// rule the load-time mark pruning applies, so every mark recorded behind +/// this check is one a reload would keep — and not this site itself, which +/// never delivers edits to itself. The caller IGNORES an inadmissible fence +/// rather than failing the request: the delivery applies exactly as an +/// unstamped (pre-fence) delivery would, no high-water mark is read or +/// written, and the worst a forged fence achieves is forfeiting an ordering +/// guarantee its sender was never owed. The generation itself is NOT +/// bounded here: a genuine origin whose hybrid clock persisted a wall-clock +/// excursion allocates arbitrarily far in the future, and refusing to +/// record its marks would strip the ordering fence from exactly the +/// deliveries that still race — the staleness window on the read side is +/// what defuses forged marks instead. +fn peer_edit_fence_is_admissible(state: &SiteReplicationState, local_deployment_id: &str, fence: &(String, u64)) -> bool { + let (origin, generation) = fence; + if origin != local_deployment_id && state.peers.contains_key(origin) { + return true; + } + warn!( + event = EVENT_ADMIN_SITE_REPLICATION_STATE, + component = LOG_COMPONENT_ADMIN, + subsystem = LOG_SUBSYSTEM_SITE_REPLICATION, + result = "fence_origin_not_a_remote_peer", + origin = %origin, + generation = *generation, + "ignoring inadmissible peer-edit fence" + ); + false +} + /// True when a strictly newer edit from the same origin site already landed /// here. No lock on the sending side can order deliveries issued by two /// nodes of that site, so ordering is decided here, on the generation the @@ -6070,11 +6119,42 @@ fn peer_edit_fence(queries: &HashMap) -> Option<(String, u64)> { /// stale: one edit legitimately fans out several deliveries under a single /// generation (the ILM-expiry edit sends every peer's record), and a replay of /// an applied delivery re-applies the same edit idempotently. +/// +/// A mark more than [`PEER_EDIT_FENCE_STALENESS_WINDOW_NANOS`] above the +/// delivery is implausible and does NOT fence: the shared service account +/// means any peer can stamp any origin, so a forged `u64::MAX`-scale mark +/// would otherwise silently swallow the origin's genuine edits for good. +/// Bounding the fence by distance instead of by an absolute ceiling keeps +/// ordering intact wherever the origin's clock actually operates — two +/// racing deliveries trail each other by seconds whether the hybrid clock +/// tracks wall time or persists a long-gone excursion far ahead of it — +/// while a mark no genuine race can explain merely downgrades the origin to +/// unfenced (pre-fence) delivery instead of dropping its edits. (One genuine +/// shape does land out here: a plain-counter straggler arriving after its +/// origin's first hybrid-clock edit. It gets the same downgrade — applied +/// unfenced — once, at upgrade time; fencing it instead would silence the +/// mirror case, a hybrid-clock origin downgraded back to the plain counter.) fn peer_edit_delivery_is_stale(state: &SiteReplicationState, origin: &str, generation: u64) -> bool { - state - .applied_edit_generations - .get(origin) - .is_some_and(|applied| *applied > generation) + let Some(applied) = state.applied_edit_generations.get(origin) else { + return false; + }; + if *applied <= generation { + return false; + } + if *applied - generation > PEER_EDIT_FENCE_STALENESS_WINDOW_NANOS { + warn!( + event = EVENT_ADMIN_SITE_REPLICATION_STATE, + component = LOG_COMPONENT_ADMIN, + subsystem = LOG_SUBSYSTEM_SITE_REPLICATION, + result = "fence_mark_beyond_staleness_window", + origin, + generation, + applied_mark = *applied, + "ignoring implausibly distant peer-edit high-water mark" + ); + return false; + } + true } fn record_applied_peer_edit_generation(state: &mut SiteReplicationState, origin: &str, generation: u64) { @@ -10698,6 +10778,11 @@ impl Operation for SRPeerEditHandler { let outcome = update_site_replication_state_when_changed(move |state| { let mut incoming = incoming; let local_peer = local_peer_at_endpoint(commit_endpoint, state); + // The fence is self-reported — the shared service account means + // the sender cannot be identified — so it is honoured only after + // the admissibility check, against the same state it will gate. + let commit_fence = + commit_fence.filter(|fence| peer_edit_fence_is_admissible(state, &local_peer.deployment_id, fence)); // Ordering fence: the sending site allocates the generation under // its state-object lock, so a delivery that lost the race carries // a generation this site has already passed. Applying it would @@ -13393,6 +13478,15 @@ mod tests { handler_block.contains("record_applied_peer_edit_generation(state, origin, *generation);"), "SRPeerEditHandler must record the applied generation so later stale deliveries are recognised" ); + // Fence hardening: origin and generation are self-reported by a + // caller the shared service account cannot identify, so the handler + // must pass the fence through the admissibility check — against the + // same state the fence gates, i.e. inside the transaction — before + // reading or raising any high-water mark. + assert!( + handler_block.contains(".filter(|fence| peer_edit_fence_is_admissible(state, &local_peer.deployment_id, fence))"), + "SRPeerEditHandler must admit a fence only through peer_edit_fence_is_admissible inside the state transaction" + ); // P1-15 PR2: both halves of the fence and the edit they fence share // ONE transaction. Checking the fence against a state read outside the // lock would let the check pass on one snapshot and the write land on @@ -14769,6 +14863,121 @@ mod tests { assert!(peer_edit_delivery_is_stale(&state, origin, generation - 1)); } + /// A fence is self-reported: every site authenticates peer traffic with + /// the same site-replicator credential, so a compromised peer can stamp + /// ANY origin with ANY generation. An origin the receiver does not + /// replicate with — or the receiver itself — is ignored and plants no + /// mark; a mark a compromised peer plants for a CURRENT origin cannot + /// silence that origin, because the staleness window refuses to fence on + /// a mark implausibly far above the genuine deliveries. + #[test] + fn forged_peer_edit_fences_cannot_poison_the_high_water_marks() { + let mut state = SiteReplicationState { + peers: BTreeMap::from([ + ( + "site-local".to_string(), + PeerInfo { + deployment_id: "site-local".to_string(), + ..peer("local", "https://local.example:9000") + }, + ), + ( + "site-victim".to_string(), + PeerInfo { + deployment_id: "site-victim".to_string(), + ..peer("victim", "https://victim.example:9000") + }, + ), + ]), + ..Default::default() + }; + // An origin outside the current membership is refused outright... + let unknown = ("site-unknown".to_string(), 4u64); + assert!(!peer_edit_fence_is_admissible(&state, "site-local", &unknown)); + + // No site delivers edits to itself: a fence claiming the receiver as + // its origin is forged by construction, current peer or not. + let own = ("site-local".to_string(), 4u64); + assert!(!peer_edit_fence_is_admissible(&state, "site-local", &own)); + + // A current remote peer's fence is admitted and works end to end. + let genuine = ("site-victim".to_string(), 1u64); + assert!(peer_edit_fence_is_admissible(&state, "site-local", &genuine)); + assert!(!peer_edit_delivery_is_stale(&state, &genuine.0, genuine.1)); + record_applied_peer_edit_generation(&mut state, &genuine.0, genuine.1); + assert_eq!(state.applied_edit_generations.get("site-victim"), Some(&1)); + + // A forged u64::MAX-scale mark CAN be recorded — the shared service + // account means the receiver cannot tell the stamp was forged — but + // it is inert: the victim's genuine hybrid-clock deliveries sit far + // more than the staleness window below it, so they keep applying + // instead of being silently acked-and-dropped. + record_applied_peer_edit_generation(&mut state, "site-victim", u64::MAX); + assert!(!peer_edit_delivery_is_stale(&state, "site-victim", edit_generation_wall_clock())); + } + + /// The staleness window bounds the fence by DISTANCE from the mark, not + /// by an absolute clock ceiling, so ordering must hold wherever the + /// origin's hybrid clock actually operates. The regression that matters: + /// a temporary wall-clock excursion far in the future is persisted by + /// `next_peer_edit_generation` (`max(now, prev + 1)` never comes back + /// down), and two later edits g+1 then g can arrive in reverse order — + /// g must still be fenced, even though both generations dwarf the + /// receiver's clock. Conversely a mark further above a delivery than any + /// genuine race can explain must not fence it. + #[test] + fn peer_edit_fence_orders_a_persisted_future_clock_and_defuses_distant_marks() { + let mut state = SiteReplicationState { + peers: BTreeMap::from([( + "site-origin".to_string(), + PeerInfo { + deployment_id: "site-origin".to_string(), + ..peer("origin", "https://origin.example:9000") + }, + )]), + ..Default::default() + }; + + // The origin's clock once jumped ten years ahead; the hybrid clock + // keeps allocating from there long after the clock was corrected. + let excursion = edit_generation_wall_clock() + 10 * 365 * 24 * 60 * 60 * 1_000_000_000; + let fence = ("site-origin".to_string(), excursion + 1); + assert!(peer_edit_fence_is_admissible(&state, "site-local", &fence)); + record_applied_peer_edit_generation(&mut state, &fence.0, fence.1); + + // The reverse delivery of the race: g arrives after g+1 landed. + // Without the fence it would commit last and roll g+1 back. + assert!(peer_edit_delivery_is_stale(&state, "site-origin", excursion)); + // Equal generation (same edit's fan-out or a replay) still applies, + // as does the next edit. + assert!(!peer_edit_delivery_is_stale(&state, "site-origin", excursion + 1)); + assert!(!peer_edit_delivery_is_stale(&state, "site-origin", excursion + 2)); + + // The window's exact boundary: a delivery trailing the mark by the + // full window is still fenced; one nanosecond further is not — that + // distance is no longer explicable by a genuine race, only by a + // forged mark or an excursion the origin has left behind. + let mark = fence.1; + // A straggler trailing by a concrete hour must still be fenced — + // pins the window's real magnitude, not just its symbolic boundary. + assert!(peer_edit_delivery_is_stale(&state, "site-origin", mark - 60 * 60 * 1_000_000_000)); + assert!(peer_edit_delivery_is_stale( + &state, + "site-origin", + mark - PEER_EDIT_FENCE_STALENESS_WINDOW_NANOS + )); + assert!(!peer_edit_delivery_is_stale( + &state, + "site-origin", + mark - PEER_EDIT_FENCE_STALENESS_WINDOW_NANOS - 1 + )); + + // A pre-hybrid plain-counter origin trails such a mark by eons: it + // is not fenced (the rc.2-era downgrade case), it just runs + // unfenced until its counter regime catches up. + assert!(!peer_edit_delivery_is_stale(&state, "site-origin", 3)); + } + /// P1-15 review follow-up: a site that leaves the mesh drops below two /// peers, which clears its state object and restarts its generation /// counter at zero. A mark left over from its previous membership would From 59b7d13095e780fccbff829a19770a713f593884 Mon Sep 17 00:00:00 2001 From: houseme Date: Mon, 17 Aug 2026 19:40:56 +0800 Subject: [PATCH 10/10] feat(scanner): expose prefix-level bucket usage via admin API (HS-08) (#6171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit feat(scanner): expose prefix-level bucket usage via admin API The scanner's per-bucket, per-set usage caches already hold a path-keyed prefix tree, but dui() flattened it only to bucket names — consoles and operators had no way to ask "what does this prefix hold" without an S3 listing sweep (rustfs/backlog#1872, MinIO loadPrefixUsageFromBackend parity). Add: - data-usage: prefix_usage_in_cache — a shared aggregation over the entry map (arbitrary prefix, full counters, one-level sub-prefix breakdown with names recovered from the literal-path cache keys), hardened like the scanner's checked flatten: cycles, dangling child links, over-deep trees, and overflowing counters yield None rather than unbounded recursion or wrapped totals. - ecstore: ECStore::all_set_disks — iterate every erasure set so a query can read each set's own cache copy; the hash-routed store path would always land on one set. - scanner: bucket_prefix_usage — per-set loads (5s budget each, a slow set degrades to not-reporting instead of stalling the caller), merged across sets with partial/compacted/truncated flags, served from a bounded 30s cache (128 entries, hard-capped) that bucket writes invalidate through the dirty-usage hook. - admin: GET /rustfs/admin/v3/usage/{bucket}?prefix=&max-entries= behind the same any-of gate as datausageinfo (DataUsageInfoAdminAction OR ListBucketAction), rejecting unknown query parameters and clamping max-entries to 1..=10000. Route registered in the policy table (deferred MultipleActions, matching datausageinfo) and the route matrix test. Closes rustfs/backlog#1872. Co-authored-by: heihutu --- crates/data-usage/src/data_usage.rs | 286 ++++++++++++++++ crates/ecstore/src/store/mod.rs | 10 + crates/scanner/src/data_usage_define.rs | 10 +- crates/scanner/src/lib.rs | 2 + crates/scanner/src/prefix_usage.rs | 349 ++++++++++++++++++++ crates/scanner/src/scanner_io.rs | 4 + rustfs/src/admin/handlers/mod.rs | 1 + rustfs/src/admin/handlers/system.rs | 8 +- rustfs/src/admin/handlers/usage_prefix.rs | 142 ++++++++ rustfs/src/admin/mod.rs | 4 +- rustfs/src/admin/route_policy.rs | 5 + rustfs/src/admin/route_registration_test.rs | 1 + 12 files changed, 816 insertions(+), 6 deletions(-) create mode 100644 crates/scanner/src/prefix_usage.rs create mode 100644 rustfs/src/admin/handlers/usage_prefix.rs diff --git a/crates/data-usage/src/data_usage.rs b/crates/data-usage/src/data_usage.rs index ebbd03261..b08281d0f 100644 --- a/crates/data-usage/src/data_usage.rs +++ b/crates/data-usage/src/data_usage.rs @@ -870,6 +870,157 @@ pub struct DataUsageCacheInfo { pub snapshot_complete: bool, } +/// Prefix-level usage over a raw entry map — the shared core behind +/// [`DataUsageCache::prefix_usage`], usable by any cache-shaped reader (the +/// scanner's writer-side cache has the same map type). +/// +/// Cache keys are cleaned literal paths (`bucket/pre/fix`), so sub-prefix +/// names come straight off the child keys — no reverse mapping exists or is +/// needed. A compacted prefix carries its aggregate but no children, which +/// the `compacted` flag reports so callers can say why the breakdown is +/// empty. `truncated` is set when the breakdown exceeded `max_entries` and +/// was cut (largest first). +pub fn prefix_usage_in_cache( + cache: &HashMap, + bucket: &str, + prefix: &str, + max_entries: usize, +) -> Option { + let prefix = prefix.trim_matches('/'); + let root = if prefix.is_empty() { + bucket.to_string() + } else { + format!("{bucket}/{prefix}") + }; + let entry = cache.get(&hash_path(&root).key())?.clone(); + + let usage = PrefixUsageSummary::from_entry(&flatten_entry(cache, &entry, 0)?); + + let child_prefix = format!("{root}/"); + let mut sub_prefixes: Vec = entry + .children + .iter() + .filter_map(|child_key| { + let child = cache.get(child_key)?; + let child_flat = flatten_entry(cache, child, 1)?; + // Child keys are literal `bucket/pre/name` paths; a trailing + // slash marks a directory object and is display-only here. + let name = child_key + .strip_prefix(child_prefix.as_str()) + .unwrap_or(child_key.as_str()) + .trim_end_matches('/') + .to_string(); + Some(PrefixUsageEntry { + prefix: name, + usage: PrefixUsageSummary::from_entry(&child_flat), + }) + }) + .collect(); + sub_prefixes.sort_by(|left, right| { + right + .usage + .size + .cmp(&left.usage.size) + .then_with(|| left.prefix.cmp(&right.prefix)) + }); + let truncated = sub_prefixes.len() > max_entries; + sub_prefixes.truncate(max_entries); + + Some(PrefixUsageQuery { + usage, + compacted: entry.compacted, + truncated, + sub_prefixes, + }) +} + +/// Maximum subtree depth [`flatten_entry`] will walk before declaring the +/// cache corrupt — the same bound the scanner's checked flatten uses. +const PREFIX_USAGE_MAX_DEPTH: usize = 1024; + +/// Flatten one entry's subtree into an aggregate: the free-function twin of +/// [`DataUsageCache::flatten`], carrying the scanner checked-flatten +/// hardening so a corrupt cache (cycles, over-deep trees, overflowing +/// counters) yields `None` instead of unbounded recursion or wrapped totals. +fn flatten_entry(cache: &HashMap, root: &DataUsageEntry, depth: usize) -> Option { + if depth > PREFIX_USAGE_MAX_DEPTH { + return None; + } + let mut flattened = DataUsageEntry::default(); + if !flattened.checked_merge(root) { + return None; + } + flattened.compacted = root.compacted; + // The root itself is not pre-seeded: it is merged above, and a corrupt + // child edge pointing back at the root's own key is still terminated by + // the visited set on first encounter. + let mut visited: HashSet<&str> = HashSet::new(); + let mut pending: Vec<(&String, usize)> = root.children.iter().map(|child| (child, depth + 1)).collect(); + while let Some((key, child_depth)) = pending.pop() { + if child_depth > PREFIX_USAGE_MAX_DEPTH || !visited.insert(key.as_str()) { + return None; + } + let entry = cache.get(key)?; + if !flattened.checked_merge(entry) { + return None; + } + pending.extend(entry.children.iter().map(|child| (child, child_depth + 1))); + } + flattened.children.clear(); + Some(flattened) +} + +/// Flattened counters of one prefix subtree, as returned by +/// [`DataUsageCache::prefix_usage`]. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "camelCase")] +pub struct PrefixUsageSummary { + pub size: u64, + pub objects: u64, + pub versions: u64, + pub delete_markers: u64, +} + +impl PrefixUsageSummary { + fn from_entry(entry: &DataUsageEntry) -> Self { + Self { + size: entry.size as u64, + objects: entry.objects as u64, + versions: entry.versions as u64, + delete_markers: entry.delete_markers as u64, + } + } + + /// Add another set's counters into this one (entries are partitioned by + /// set, so per-set results sum). + pub fn merge(&mut self, other: &Self) { + self.size = self.size.saturating_add(other.size); + self.objects = self.objects.saturating_add(other.objects); + self.versions = self.versions.saturating_add(other.versions); + self.delete_markers = self.delete_markers.saturating_add(other.delete_markers); + } +} + +/// One first-level sub-prefix row of a [`PrefixUsageQuery`]. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +pub struct PrefixUsageEntry { + pub prefix: String, + pub usage: PrefixUsageSummary, +} + +/// Result of [`DataUsageCache::prefix_usage`]. +#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "camelCase")] +pub struct PrefixUsageQuery { + pub usage: PrefixUsageSummary, + /// The prefix entry was compacted by the scanner: its aggregate is valid + /// but no sub-prefix breakdown exists on disk. + pub compacted: bool, + /// The breakdown had more entries than `max_entries`; the largest remain. + pub truncated: bool, + pub sub_prefixes: Vec, +} + /// Read-only projection of a scanner-written `.usage-cache.bin` file. /// /// The scanner-side `DataUsageCache` (`crates/scanner/src/data_usage_define.rs`) @@ -997,6 +1148,21 @@ impl DataUsageCache { } } + /// Prefix-level usage for one bucket subtree, plus the one-level + /// breakdown below it (rustfs/backlog#1872, MinIO + /// `loadPrefixUsageFromBackend` parity and beyond: arbitrary prefixes and + /// full counters instead of first-level sizes only). + /// + /// Cache keys are cleaned literal paths (`bucket/pre/fix`), so sub-prefix + /// names come straight off the child keys — no reverse mapping exists or + /// is needed. A compacted prefix carries its aggregate but no children, + /// which the `compacted` flag reports so callers can say why the + /// breakdown is empty. `truncated` is set when the breakdown exceeded + /// `max_entries` and was cut (largest first). + pub fn prefix_usage(&self, bucket: &str, prefix: &str, max_entries: usize) -> Option { + prefix_usage_in_cache(&self.cache, bucket, prefix, max_entries) + } + pub fn force_compact(&mut self, limit: usize) { if self.cache.len() < limit { return; @@ -1898,6 +2064,126 @@ mod tests { ); } + /// Build a cache shaped like `bucket/{a,b/{c,d}},bucket/loose` with + /// distinct counters so aggregation is observable. + fn prefix_usage_fixture_cache() -> DataUsageCache { + let mut cache = DataUsageCache::default(); + let mut insert = |path: &str, parent: &str, size: usize, objects: usize, versions: usize, delete_markers: usize| { + cache.replace( + path, + parent, + DataUsageEntry { + size, + objects, + versions, + delete_markers, + ..Default::default() + }, + ); + }; + insert("bucket", "", 0, 0, 0, 0); + insert("bucket/a", "bucket", 100, 1, 1, 0); + insert("bucket/b", "bucket", 0, 0, 0, 0); + insert("bucket/b/c", "bucket/b", 200, 2, 2, 1); + insert("bucket/b/d", "bucket/b", 40, 1, 3, 0); + insert("bucket/loose", "bucket", 10, 1, 1, 1); + cache + } + + #[test] + fn prefix_usage_aggregates_bucket_root_and_one_level_below() { + let cache = prefix_usage_fixture_cache(); + + let root = cache + .prefix_usage("bucket", "", 100) + .expect("root query must find the bucket entry"); + assert_eq!(root.usage.size, 350, "root aggregate flattens the whole subtree"); + assert_eq!(root.usage.objects, 5); + assert_eq!(root.usage.versions, 7); + assert_eq!(root.usage.delete_markers, 2); + assert!(!root.compacted); + assert!(!root.truncated); + // Breakdown is one level: b (240) before a (100) before loose (10), + // each flattened to its own subtree total. + let names: Vec<(&str, u64)> = root + .sub_prefixes + .iter() + .map(|entry| (entry.prefix.as_str(), entry.usage.size)) + .collect(); + assert_eq!(names, vec![("b", 240), ("a", 100), ("loose", 10)]); + } + + #[test] + fn prefix_usage_drills_into_arbitrary_prefixes() { + let cache = prefix_usage_fixture_cache(); + + let b = cache.prefix_usage("bucket", "b", 100).expect("nested prefix must resolve"); + assert_eq!(b.usage.size, 240); + assert_eq!(b.usage.versions, 5); + let names: Vec<&str> = b.sub_prefixes.iter().map(|entry| entry.prefix.as_str()).collect(); + assert_eq!(names, vec!["c", "d"]); + + // Prefix slashes are normalized away. + let slashed = cache.prefix_usage("bucket", "/b/", 100).expect("slash-insensitive lookup"); + assert_eq!(slashed.usage.size, 240); + + assert!(cache.prefix_usage("bucket", "absent", 100).is_none(), "unknown prefix must be a miss"); + assert!(cache.prefix_usage("other", "", 100).is_none(), "unknown bucket must be a miss"); + } + + #[test] + fn prefix_usage_reports_and_respects_truncation() { + let cache = prefix_usage_fixture_cache(); + let capped = cache.prefix_usage("bucket", "", 2).expect("root query"); + assert!(capped.truncated, "three children capped to two must flag truncation"); + let names: Vec<&str> = capped.sub_prefixes.iter().map(|entry| entry.prefix.as_str()).collect(); + assert_eq!(names, vec!["b", "a"], "largest prefixes survive the cut"); + } + + #[test] + fn prefix_usage_marks_compacted_entries() { + let mut cache = DataUsageCache::default(); + cache.replace( + "bucket", + "", + DataUsageEntry { + size: 999, + objects: 9, + compacted: true, + ..Default::default() + }, + ); + + let compacted = cache.prefix_usage("bucket", "", 100).expect("compacted root resolves"); + assert!(compacted.compacted, "compaction must be visible to callers"); + assert_eq!(compacted.usage.size, 999); + assert!(compacted.sub_prefixes.is_empty(), "a compacted entry carries no children"); + } + + #[test] + fn prefix_usage_rejects_cyclic_and_dangling_caches() { + // A self-referencing child (corrupt cache) must yield a miss for the + // whole query, not unbounded recursion. + let mut cache = prefix_usage_fixture_cache(); + if let Some(entry) = cache.cache.get_mut("bucket/b") { + entry.children.insert("bucket/b".to_string()); + } + assert!(cache.prefix_usage("bucket", "b", 100).is_none(), "a cyclic subtree must be rejected"); + // The unaffected sibling still answers. + assert!(cache.prefix_usage("bucket", "a", 100).is_some()); + + // A child key with no entry (dangling link) is rejected rather than + // silently dropped: half a tree would under-report usage. + let mut dangling = prefix_usage_fixture_cache(); + if let Some(entry) = dangling.cache.get_mut("bucket/b") { + entry.children.insert("bucket/b/ghost".to_string()); + } + assert!( + dangling.prefix_usage("bucket", "b", 100).is_none(), + "a dangling child link must be rejected" + ); + } + #[test] fn hash_path_uses_portable_slash_semantics() { for (input, expected) in [ diff --git a/crates/ecstore/src/store/mod.rs b/crates/ecstore/src/store/mod.rs index f51fa6df5..8d02ef441 100644 --- a/crates/ecstore/src/store/mod.rs +++ b/crates/ecstore/src/store/mod.rs @@ -216,6 +216,16 @@ impl std::fmt::Debug for ECStore { /// These delegate to the process-global statics. No local state — the globals /// remain the single source of truth until the migration is complete. impl ECStore { + /// Every erasure set across all pools, pool-major order. + /// + /// Read-only queries that must consult each set's own copy of a + /// per-bucket object (e.g. the scanner's `.usage-cache.bin`) iterate + /// this instead of the hash-routed store path, which would always land + /// on one set (rustfs/backlog#1872). + pub fn all_set_disks(&self) -> Vec> { + self.pools.iter().flat_map(|pool| pool.disk_set.iter().cloned()).collect() + } + /// Get server configuration (delegates to global) pub fn get_server_config(&self) -> Option { runtime_sources::server_config() diff --git a/crates/scanner/src/data_usage_define.rs b/crates/scanner/src/data_usage_define.rs index 67a199661..8ac714314 100644 --- a/crates/scanner/src/data_usage_define.rs +++ b/crates/scanner/src/data_usage_define.rs @@ -28,7 +28,8 @@ use rustfs_common::heal_channel::HealScanMode; use rustfs_config::ENV_SCANNER_CACHE_SAVE_TIMEOUT_SECS; pub use rustfs_data_usage::{ AllTierStats, BucketTargetUsageInfo, BucketUsageInfo, DATA_USAGE_OBJECT_NAME, DATA_USAGE_OBSERVED_OBJECT_NAME, - DataUsageEntry, DataUsageHash, DataUsageHashMap, DataUsageInfo, LEGACY_DATA_USAGE_OBJECT_NAME, TierStats, hash_path, + DataUsageEntry, DataUsageHash, DataUsageHashMap, DataUsageInfo, LEGACY_DATA_USAGE_OBJECT_NAME, PrefixUsageEntry, + PrefixUsageQuery, PrefixUsageSummary, TierStats, hash_path, prefix_usage_in_cache, }; use rustfs_utils::path::{SLASH_SEPARATOR, path_join_buf}; use tokio::time::{Duration, Instant, sleep, timeout}; @@ -430,6 +431,13 @@ pub(crate) enum DataUsageCachePrepareOutcome { } impl DataUsageCache { + /// Prefix-level usage query over this (writer-side) cache; see + /// [`prefix_usage_in_cache`] for the semantics + /// (rustfs/backlog#1872). + pub fn prefix_usage(&self, bucket: &str, prefix: &str, max_entries: usize) -> Option { + prefix_usage_in_cache(&self.cache, bucket, prefix, max_entries) + } + pub(crate) fn prepare_for_scan( &mut self, name: &str, diff --git a/crates/scanner/src/lib.rs b/crates/scanner/src/lib.rs index 4a5cc7543..36cc6b817 100644 --- a/crates/scanner/src/lib.rs +++ b/crates/scanner/src/lib.rs @@ -53,6 +53,7 @@ use tokio_util::sync::CancellationToken; pub mod data_usage_define; pub mod error; +pub mod prefix_usage; mod remote_scanner; pub mod runtime_config; pub mod scanner; @@ -64,6 +65,7 @@ pub(crate) mod storage_api; pub use data_usage_define::*; pub use error::ScannerError; +pub use prefix_usage::{BucketPrefixUsageResponse, bucket_prefix_usage, invalidate_prefix_usage_cache}; pub use remote_scanner::{ NS_SCANNER_MAX_REQUEST_BODY_SIZE, RemoteScannerAdmission, RemoteScannerRequest, admit_remote_scanner_request, claim_remote_scanner_request, decode_remote_scanner_request, preflight_remote_scanner_request, diff --git a/crates/scanner/src/prefix_usage.rs b/crates/scanner/src/prefix_usage.rs new file mode 100644 index 000000000..9f94957de --- /dev/null +++ b/crates/scanner/src/prefix_usage.rs @@ -0,0 +1,349 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Prefix-level bucket usage for admin/console consumers (rustfs/backlog#1872, +//! MinIO `loadPrefixUsageFromBackend` parity). +//! +//! The per-bucket, per-set `.usage-cache.bin` objects already hold a +//! path-keyed prefix tree; this module reads every set's copy through that +//! set's own object layer (the hash-routed store path would always land on +//! one set), aggregates the overlapping trees, and serves the result from a +//! bounded 30-second cache. Bucket writes poke the cache through the +//! dirty-usage hook so a fresh scan is visible immediately. + +use crate::data_usage_define::{DATA_USAGE_CACHE_NAME, DataUsageCache}; +use crate::error::ScannerError; +use crate::storage_api::owner::{ + EcstoreSetDisks, EcstoreStore, ecstore_is_reserved_or_invalid_bucket, ecstore_resolve_object_store_handle, +}; +use futures::future::join_all; +use rustfs_data_usage::{PrefixUsageEntry, PrefixUsageSummary}; +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, SystemTime}; +use tracing::{debug, warn}; + +const LOG_COMPONENT_SCANNER: &str = "scanner"; +const LOG_SUBSYSTEM_PREFIX_USAGE: &str = "prefix_usage"; +const EVENT_PREFIX_USAGE_CACHE_STATE: &str = "prefix_usage_cache_state"; + +/// How long a computed breakdown stays fresh. MinIO uses the same 30s for +/// its prefix-usage cache; bucket writes additionally invalidate on the spot. +const CACHE_TTL: Duration = Duration::from_secs(30); +/// Hard entry cap for the result cache; exceeded, expired entries go first +/// and the map clears rather than growing past the bound. +const CACHE_MAX_ENTRIES: usize = 128; +/// Per-set cache read budget. The underlying loader retries for up to a +/// minute per attempt on backend errors — far too long for an admin GET, so +/// a slow set degrades to "not reporting" instead of stalling the caller. +const PER_SET_LOAD_TIMEOUT: Duration = Duration::from_secs(5); + +/// Aggregated prefix-usage answer across every erasure set. +#[derive(Clone, Debug, PartialEq, serde::Serialize)] +#[serde(rename_all = "camelCase")] +pub struct BucketPrefixUsageResponse { + pub bucket: String, + pub prefix: String, + pub usage: PrefixUsageSummary, + /// Every reporting set's prefix entry was compacted: the aggregate is + /// valid, the sub-prefix breakdown is empty on disk. + pub compacted: bool, + /// The sub-prefix breakdown is incomplete: at least one reporting set + /// had the prefix compacted (or absent while others found it), so its + /// objects cannot be attributed to a sub-prefix. + pub sub_prefixes_partial: bool, + /// The breakdown exceeded the caller's entry limit; largest remain. + pub truncated: bool, + pub sub_prefixes: Vec, + /// Sets whose cache held this bucket and prefix. + pub sets_reporting: usize, + pub sets_total: usize, + /// Newest `last_update` across reporting sets, unix seconds. + pub last_update_unix_secs: Option, +} + +#[derive(Clone)] +struct CachedResponse { + computed_at: std::time::Instant, + response: Arc, +} + +/// Cache key: (lowercased bucket, normalized prefix, max entries). +type PrefixUsageCacheKey = (String, String, usize); +type PrefixUsageCacheMap = Option>; + +static PREFIX_USAGE_CACHE: Mutex = Mutex::new(None); + +/// Drop cached results for `bucket` (empty string clears everything). Wired +/// into the dirty-usage recording path so a write makes the next prefix +/// query recompute instead of serving up to `CACHE_TTL` seconds of stale +/// numbers. +pub fn invalidate_prefix_usage_cache(bucket: &str) { + let mut guard = PREFIX_USAGE_CACHE.lock().unwrap_or_else(|poison| poison.into_inner()); + let Some(map) = guard.as_mut() else { + return; + }; + if bucket.is_empty() { + map.clear(); + return; + } + map.retain(|(cached_bucket, ..), _| !cached_bucket.eq_ignore_ascii_case(bucket)); +} + +/// Query prefix usage for `bucket` (arbitrary `prefix`, empty = whole +/// bucket), merging every erasure set's own cache copy. `max_entries` bounds +/// the sub-prefix rows (largest first). +pub async fn bucket_prefix_usage( + bucket: &str, + prefix: &str, + max_entries: usize, +) -> Result { + if ecstore_is_reserved_or_invalid_bucket(bucket, true) { + return Err(ScannerError::Other(format!("invalid bucket name: {bucket}"))); + } + let normalized_prefix = prefix.trim_matches('/').to_string(); + let cache_key = (bucket.to_ascii_lowercase(), normalized_prefix.clone(), max_entries); + if let Some(response) = lookup_cached(&cache_key) { + return Ok((*response).clone()); + } + + let store = ecstore_resolve_object_store_handle() + .ok_or_else(|| ScannerError::Other("object store is not initialized".to_string()))?; + let response = Arc::new(compute_prefix_usage(store, bucket, &normalized_prefix, max_entries).await); + store_cached(cache_key, response.clone()); + Ok((*response).clone()) +} + +async fn compute_prefix_usage( + store: Arc, + bucket: &str, + prefix: &str, + max_entries: usize, +) -> BucketPrefixUsageResponse { + let sets: Vec> = store.all_set_disks(); + let sets_total = sets.len(); + let cache_name = format!("{bucket}/{DATA_USAGE_CACHE_NAME}"); + + let per_set = join_all(sets.into_iter().map(|set| { + let cache_name = cache_name.clone(); + async move { + let mut cache = DataUsageCache::default(); + // A set that has never scanned this bucket (or cannot be read + // within the budget) reports nothing — the remaining sets still + // produce a usable, flagged answer. + let loaded = match tokio::time::timeout(PER_SET_LOAD_TIMEOUT, cache.load(set, &cache_name)).await { + Ok(Ok(())) => cache, + Ok(Err(err)) => { + debug!( + target: "rustfs::scanner::prefix_usage", + event = EVENT_PREFIX_USAGE_CACHE_STATE, + component = LOG_COMPONENT_SCANNER, + subsystem = LOG_SUBSYSTEM_PREFIX_USAGE, + bucket = %bucket, + state = "set_load_failed", + error = %err, + "Prefix usage set cache load failed" + ); + return None; + } + Err(_) => { + warn!( + target: "rustfs::scanner::prefix_usage", + event = EVENT_PREFIX_USAGE_CACHE_STATE, + component = LOG_COMPONENT_SCANNER, + subsystem = LOG_SUBSYSTEM_PREFIX_USAGE, + bucket = %bucket, + state = "set_load_timeout", + "Prefix usage set cache load timed out" + ); + return None; + } + }; + if loaded.info.name != bucket { + // Empty or stale-scoped cache: this set has no data for the bucket. + return None; + } + let last_update = loaded.info.last_update; + let query = loaded.prefix_usage(bucket, prefix, max_entries); + Some((query, last_update)) + } + })) + .await; + + let mut usage = PrefixUsageSummary::default(); + let mut sub_prefix_map: HashMap = HashMap::new(); + let mut sets_reporting = 0usize; + let mut reporting_but_absent = 0usize; + let mut any_compacted = false; + let mut all_compacted = true; + let mut truncated = false; + let mut last_update: Option = None; + + for (query, set_last_update) in per_set.into_iter().flatten() { + // last_update counts every set that has scanned the bucket, even + // when the prefix itself is absent on that set. + if let Some(set_last_update) = set_last_update + && last_update.map(|current| set_last_update > current).unwrap_or(true) + { + last_update = Some(set_last_update); + } + let Some(query) = query else { + // The set knows the bucket but not this prefix: legitimate when + // the prefix's objects all hash to other sets, but it means the + // breakdown below cannot attribute that set's (zero) objects. + reporting_but_absent += 1; + continue; + }; + sets_reporting += 1; + usage.merge(&query.usage); + if query.compacted { + any_compacted = true; + } else { + all_compacted = false; + } + truncated |= query.truncated; + for entry in query.sub_prefixes { + sub_prefix_map.entry(entry.prefix).or_default().merge(&entry.usage); + } + } + + let mut sub_prefixes: Vec = sub_prefix_map + .into_iter() + .map(|(prefix, usage)| PrefixUsageEntry { prefix, usage }) + .collect(); + sub_prefixes.sort_by(|left, right| { + right + .usage + .size + .cmp(&left.usage.size) + .then_with(|| left.prefix.cmp(&right.prefix)) + }); + // Merged rows can exceed max_entries only when per-set truncation + // already flagged; enforce the caller bound on the merged view too. + if sub_prefixes.len() > max_entries { + truncated = true; + sub_prefixes.truncate(max_entries); + } + + let found = sets_reporting > 0; + BucketPrefixUsageResponse { + bucket: bucket.to_string(), + prefix: prefix.to_string(), + usage, + compacted: found && all_compacted, + sub_prefixes_partial: any_compacted || reporting_but_absent > 0, + truncated, + sub_prefixes, + sets_reporting, + sets_total, + last_update_unix_secs: last_update + .and_then(|time| time.duration_since(SystemTime::UNIX_EPOCH).ok()) + .map(|dur| dur.as_secs()), + } +} + +fn lookup_cached(key: &(String, String, usize)) -> Option> { + let mut guard = PREFIX_USAGE_CACHE.lock().unwrap_or_else(|poison| poison.into_inner()); + let map = guard.as_mut()?; + let cached = map.get(key)?; + if cached.computed_at.elapsed() > CACHE_TTL { + map.remove(key); + return None; + } + Some(cached.response.clone()) +} + +fn store_cached(key: (String, String, usize), response: Arc) { + let mut guard = PREFIX_USAGE_CACHE.lock().unwrap_or_else(|poison| poison.into_inner()); + let map = guard.get_or_insert_with(HashMap::new); + // Bound the cache: drop expired entries first, and if the cap is still + // exceeded clear wholesale — the next queries recompute in milliseconds. + if map.len() >= CACHE_MAX_ENTRIES { + map.retain(|_, cached| cached.computed_at.elapsed() <= CACHE_TTL); + if map.len() >= CACHE_MAX_ENTRIES { + map.clear(); + } + } + map.insert( + key, + CachedResponse { + computed_at: std::time::Instant::now(), + response, + }, + ); +} + +#[cfg(test)] +mod tests { + use super::{CACHE_MAX_ENTRIES, PREFIX_USAGE_CACHE, invalidate_prefix_usage_cache, store_cached}; + use rustfs_data_usage::PrefixUsageSummary; + + fn response(bucket: &str) -> super::BucketPrefixUsageResponse { + super::BucketPrefixUsageResponse { + bucket: bucket.to_string(), + prefix: String::new(), + usage: PrefixUsageSummary::default(), + compacted: false, + sub_prefixes_partial: false, + truncated: false, + sub_prefixes: Vec::new(), + sets_reporting: 1, + sets_total: 1, + last_update_unix_secs: None, + } + } + + fn seed(bucket: &str, prefix: &str) { + store_cached( + (bucket.to_ascii_lowercase(), prefix.to_string(), 10), + std::sync::Arc::new(response(bucket)), + ); + } + + fn contains(bucket: &str, prefix: &str) -> bool { + PREFIX_USAGE_CACHE + .lock() + .unwrap_or_else(|poison| poison.into_inner()) + .as_ref() + .is_some_and(|map| map.contains_key(&(bucket.to_ascii_lowercase(), prefix.to_string(), 10))) + } + + /// All cache tests run inside one test to keep the process-global map + /// free of cross-test ordering (the flake class this module avoids). + #[test] + fn invalidation_scopes_to_bucket_and_cache_stays_bounded() { + invalidate_prefix_usage_cache(""); + seed("alpha", "x"); + seed("beta", "y"); + + // Case-insensitive bucket scoping. + invalidate_prefix_usage_cache("ALPHA"); + assert!(!contains("alpha", "x")); + assert!(contains("beta", "y")); + + // Wholesale clear. + invalidate_prefix_usage_cache(""); + assert!(!contains("beta", "y")); + + // Hard cap: overflow clears rather than grows. + for index in 0..=(CACHE_MAX_ENTRIES / 2) { + let bucket = format!("cap-bucket-{index}"); + seed(&bucket, "a"); + seed(&bucket, "b"); + } + let guard = PREFIX_USAGE_CACHE.lock().unwrap_or_else(|poison| poison.into_inner()); + let map = guard.as_ref().expect("seeded"); + assert!(map.len() <= CACHE_MAX_ENTRIES, "cache must stay bounded, got {}", map.len()); + } +} diff --git a/crates/scanner/src/scanner_io.rs b/crates/scanner/src/scanner_io.rs index 15ee9cca0..f722ff186 100644 --- a/crates/scanner/src/scanner_io.rs +++ b/crates/scanner/src/scanner_io.rs @@ -231,6 +231,10 @@ pub fn record_dirty_usage_bucket(bucket: &str) { dirty_buckets.len() }; global_metrics().record_scanner_dirty_usage_pending(usize_to_u64_saturated(pending_buckets)); + // A write invalidates this bucket's prefix-usage answers on the spot so + // admin/console consumers never ride the full TTL after a change + // (rustfs/backlog#1872). + crate::prefix_usage::invalidate_prefix_usage_cache(bucket); DIRTY_USAGE_BUCKET_NOTIFY.notify_one(); } diff --git a/rustfs/src/admin/handlers/mod.rs b/rustfs/src/admin/handlers/mod.rs index 8c837eb05..f0a32f402 100644 --- a/rustfs/src/admin/handlers/mod.rs +++ b/rustfs/src/admin/handlers/mod.rs @@ -64,6 +64,7 @@ mod target_descriptor; pub mod tier; pub mod tls_debug; pub mod trace; +pub mod usage_prefix; pub mod user; pub mod user_iam; pub mod user_lifecycle; diff --git a/rustfs/src/admin/handlers/system.rs b/rustfs/src/admin/handlers/system.rs index c7ad93b54..0909568e5 100644 --- a/rustfs/src/admin/handlers/system.rs +++ b/rustfs/src/admin/handlers/system.rs @@ -1158,10 +1158,10 @@ impl Operation for RuntimeCapabilitiesHandler { } } -/// Authorization gate for GET datausageinfo: any-of the dedicated admin action -/// OR the bucket listing action. Pinned by a unit test so the gate cannot -/// silently narrow or widen (rustfs/backlog#1306). -fn data_usage_info_gate_actions() -> Vec { +/// Authorization gate for GET datausageinfo (and prefix usage): any-of the +/// dedicated admin action OR the bucket listing action. Pinned by a unit test +/// so the gate cannot silently narrow or widen (rustfs/backlog#1306). +pub(crate) fn data_usage_info_gate_actions() -> Vec { vec![ Action::AdminAction(AdminAction::DataUsageInfoAdminAction), Action::S3Action(S3Action::ListBucketAction), diff --git a/rustfs/src/admin/handlers/usage_prefix.rs b/rustfs/src/admin/handlers/usage_prefix.rs new file mode 100644 index 000000000..10cce50d5 --- /dev/null +++ b/rustfs/src/admin/handlers/usage_prefix.rs @@ -0,0 +1,142 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Prefix-level bucket usage admin handler (rustfs/backlog#1872). +//! +//! `GET /rustfs/admin/v3/usage/{bucket}?prefix=&max-entries=` answers +//! "what does this bucket / this prefix hold" from the scanner's per-set +//! usage caches, with a one-level sub-prefix breakdown — the data console +//! buckets view MinIO serves from `loadPrefixUsageFromBackend`. + +use crate::admin::auth::validate_admin_request; +use crate::admin::handlers::system::data_usage_info_gate_actions; +use crate::admin::router::{AdminOperation, Operation, S3Router}; +use crate::auth::{check_key_valid, get_session_token}; +use crate::server::{ADMIN_PREFIX, RemoteAddr}; +use http::{HeaderMap, HeaderValue, StatusCode}; +use hyper::Method; +use matchit::Params; +use s3s::header::CONTENT_TYPE; +use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, s3_error}; + +const JSON_CONTENT_TYPE: &str = "application/json"; +const DEFAULT_MAX_ENTRIES: usize = 1000; +const MAX_ENTRIES_LIMIT: usize = 10_000; + +pub struct BucketPrefixUsageHandler {} + +pub fn register_usage_prefix_route(r: &mut S3Router) -> std::io::Result<()> { + r.insert( + Method::GET, + format!("{}{}", ADMIN_PREFIX, "/v3/usage/{bucket}").as_str(), + AdminOperation(&BucketPrefixUsageHandler {}), + )?; + Ok(()) +} + +/// Parse `prefix` and `max-entries` from the query string. Unknown keys are +/// rejected so a typo'd parameter cannot silently change the answer's shape. +fn parse_usage_prefix_query(query: Option<&str>) -> S3Result<(String, usize)> { + let mut prefix: Option = None; + let mut max_entries: Option = None; + for (key, value) in url::form_urlencoded::parse(query.unwrap_or_default().as_bytes()) { + match key.as_ref() { + "prefix" => prefix = Some(value.into_owned()), + "max-entries" => { + max_entries = Some( + value + .parse::() + .map_err(|_| s3_error!(InvalidArgument, "max-entries must be a positive integer"))?, + ); + } + other => return Err(s3_error!(InvalidArgument, "unknown query parameter: {other}")), + } + } + let max_entries = max_entries.unwrap_or(DEFAULT_MAX_ENTRIES).clamp(1, MAX_ENTRIES_LIMIT); + Ok((prefix.unwrap_or_default(), max_entries)) +} + +#[async_trait::async_trait] +impl Operation for BucketPrefixUsageHandler { + async fn call(&self, req: S3Request, params: Params<'_, '_>) -> S3Result> { + let Some(input_cred) = req.credentials else { + return Err(s3_error!(InvalidRequest, "get cred failed")); + }; + + let (cred, owner) = + check_key_valid(get_session_token(&req.uri, &req.headers).unwrap_or_default(), &input_cred.access_key).await?; + + let remote_addr = req.extensions.get::>().and_then(|opt| opt.map(|a| a.0)); + validate_admin_request(&req.headers, &cred, owner, false, data_usage_info_gate_actions(), remote_addr).await?; + + let bucket = params.get("bucket").unwrap_or_default().to_string(); + if bucket.is_empty() { + return Err(s3_error!(InvalidRequest, "bucket path parameter is required")); + } + let (prefix, max_entries) = parse_usage_prefix_query(req.uri.query())?; + + // Authorization is bucket-scoped by the same any-of gate as the + // datausageinfo route; the bucket name itself is validated by the + // scanner layer, which rejects reserved/invalid names. + let response = rustfs_scanner::bucket_prefix_usage(&bucket, &prefix, max_entries) + .await + .map_err(|err| s3_error!(InvalidArgument, "{}", err))?; + + let data = serde_json::to_vec(&response) + .map_err(|_| S3Error::with_message(S3ErrorCode::InternalError, "parse prefix usage failed"))?; + let mut header = HeaderMap::new(); + header.insert(CONTENT_TYPE, HeaderValue::from_static(JSON_CONTENT_TYPE)); + + Ok(S3Response::with_headers((StatusCode::OK, Body::from(data)), header)) + } +} + +#[cfg(test)] +mod tests { + use super::{DEFAULT_MAX_ENTRIES, MAX_ENTRIES_LIMIT, parse_usage_prefix_query}; + use s3s::S3Error; + + fn query(raw: &str) -> Result<(String, usize), S3Error> { + parse_usage_prefix_query(Some(raw)) + } + + #[test] + fn defaults_apply_when_no_query_is_given() { + assert_eq!(parse_usage_prefix_query(None).unwrap(), (String::new(), DEFAULT_MAX_ENTRIES)); + assert_eq!(query("").unwrap(), (String::new(), DEFAULT_MAX_ENTRIES)); + } + + #[test] + fn prefix_round_trips_url_encoded_characters() { + let (prefix, _) = query("prefix=pre%2Ffix%20name").unwrap(); + assert_eq!(prefix, "pre/fix name"); + } + + #[test] + fn max_entries_parses_and_clamps_to_documented_bounds() { + assert_eq!(query("max-entries=5").unwrap().1, 5); + assert_eq!(query("max-entries=0").unwrap().1, 1, "zero must clamp up, not mean unlimited"); + assert_eq!(query("max-entries=99999999").unwrap().1, MAX_ENTRIES_LIMIT); + assert!(query("max-entries=-3").is_err()); + assert!(query("max-entries=abc").is_err()); + } + + #[test] + fn unknown_parameters_are_rejected_not_ignored() { + assert!( + query("prefixes=x").is_err(), + "a typo'd parameter must fail the request, not widen the query" + ); + } +} diff --git a/rustfs/src/admin/mod.rs b/rustfs/src/admin/mod.rs index d61abf932..fe07d4056 100644 --- a/rustfs/src/admin/mod.rs +++ b/rustfs/src/admin/mod.rs @@ -40,7 +40,8 @@ use handlers::{ audit, batch_job, bucket_meta, cluster_snapshot, config_admin, diagnostics, durability as durability_handler, extensions, heal, health, idp_compat, ilm_transition, inspect_archive, kms, module_switch, object_data_cache, object_zip_download, oidc, plugins_catalog, plugins_instances, pools, profile_admin, quota as quota_handler, rebalance, - replication as replication_handler, scanner, site_replication, sts, system, table_catalog, tier, tls_debug, user, + replication as replication_handler, scanner, site_replication, sts, system, table_catalog, tier, tls_debug, usage_prefix, + user, }; use router::{AdminOperation, S3Router}; use s3s::route::S3Route; @@ -80,6 +81,7 @@ fn register_admin_routes(r: &mut S3Router) -> std::io::Result<() bucket_meta::register_bucket_meta_route(r)?; config_admin::register_config_route(r)?; scanner::register_scanner_route(r)?; + usage_prefix::register_usage_prefix_route(r)?; ilm_transition::register_ilm_transition_route(r)?; object_data_cache::register_object_data_cache_route(r)?; audit::register_audit_target_route(r)?; diff --git a/rustfs/src/admin/route_policy.rs b/rustfs/src/admin/route_policy.rs index 474d3ad0f..2c1ad5e1b 100644 --- a/rustfs/src/admin/route_policy.rs +++ b/rustfs/src/admin/route_policy.rs @@ -1558,6 +1558,11 @@ pub const DEFERRED_ADMIN_ROUTE_POLICIES: &[DeferredAdminRoutePolicy] = &[ "/rustfs/admin/v3/datausageinfo", DeferredRoutePolicyReason::MultipleActions, ), + deferred( + HttpMethod::Get, + "/rustfs/admin/v3/usage/{bucket}", + DeferredRoutePolicyReason::MultipleActions, + ), deferred( HttpMethod::Post, "/rustfs/admin/v3/object-zip-downloads", diff --git a/rustfs/src/admin/route_registration_test.rs b/rustfs/src/admin/route_registration_test.rs index f829b8c94..e81f13d17 100644 --- a/rustfs/src/admin/route_registration_test.rs +++ b/rustfs/src/admin/route_registration_test.rs @@ -172,6 +172,7 @@ fn expected_admin_route_matrix() -> Vec { admin_route(Method::POST, "/v4/inspect/archive"), admin_route(Method::GET, "/v3/storageinfo"), admin_route(Method::GET, "/v3/datausageinfo"), + admin_route_sample(Method::GET, "/v3/usage/{bucket}", "/v3/usage/test-bucket"), admin_route(Method::GET, "/v3/metrics"), admin_route(Method::GET, "/v3/object-data-cache/stats"), admin_route(Method::POST, "/v3/object-data-cache/flush"),