From 9fb41175c2d3163ec7fc573bef1b23872f1ed258 Mon Sep 17 00:00:00 2001 From: cxymds Date: Sun, 6 Sep 2026 19:07:08 +0800 Subject: [PATCH] fix(ilm): resolve recovery disposition clippy errors --- crates/ecstore/src/api/mod.rs | 9 ++ .../src/bucket/lifecycle/durable_namespace.rs | 115 ++++++------------ .../bucket/lifecycle/recovery_disposition.rs | 8 +- rustfs/src/admin/handlers/ilm_transition.rs | 7 +- 4 files changed, 48 insertions(+), 91 deletions(-) diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index 0fbe6137a..6e4fc604d 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -76,6 +76,15 @@ pub mod bucket { }; } + pub mod recovery_disposition { + pub use crate::bucket::lifecycle::recovery_disposition::{ + CreatedIlmRecoveryDisposition, IlmRecoveryDisposition, IlmRecoveryDispositionAction, IlmRecoveryDispositionError, + IlmRecoveryDispositionIdentity, IlmRecoveryDispositionOwnerLease, IlmRecoveryDispositionReasonCode, + IlmRecoveryDispositionState, ObservedIlmRecoveryDisposition, create_recovery_disposition_if_absent, + load_recovery_disposition, recovery_disposition_id, save_recovery_disposition_if_current, + }; + } + pub mod recovery_export { pub use crate::bucket::lifecycle::recovery_export::{ IlmRecoveryExportCreated, IlmRecoveryExportObservation, create_recovery_export, diff --git a/crates/ecstore/src/bucket/lifecycle/durable_namespace.rs b/crates/ecstore/src/bucket/lifecycle/durable_namespace.rs index 9105fa3dd..eb58855f3 100644 --- a/crates/ecstore/src/bucket/lifecycle/durable_namespace.rs +++ b/crates/ecstore/src/bucket/lifecycle/durable_namespace.rs @@ -356,36 +356,7 @@ impl DurableIlmRecordCheckpoint { { return Err(Error::other("durable ILM tier delete journal checkpoint is invalid")); } - if let Self::RecoveryDisposition { - content_sha256, - identity_sha256, - copy_manifest_sha256, - copy_manifest_count, - created_at_unix_nanos, - revision, - state, - owner_fence_sha256, - owner_lease_acquired_at_unix_nanos, - owner_lease_expires_at_unix_nanos, - confirmed_absent_sha256, - retain_until_unix_nanos, - .. - } = checkpoint - && !recovery_disposition_checkpoint_is_valid( - content_sha256, - identity_sha256, - copy_manifest_sha256, - *copy_manifest_count, - *created_at_unix_nanos, - *revision, - state.clone(), - owner_fence_sha256.as_deref(), - *owner_lease_acquired_at_unix_nanos, - *owner_lease_expires_at_unix_nanos, - confirmed_absent_sha256, - *retain_until_unix_nanos, - ) - { + if !recovery_disposition_checkpoint_is_valid(checkpoint) { return Err(Error::other("durable ILM recovery disposition checkpoint is invalid")); } } @@ -761,35 +732,7 @@ impl DurableIlmRecordCheckpoint { /// purge older object versions exposed by that deletion. pub(crate) fn is_predecessor_of_terminal(&self, terminal: &Self) -> bool { for checkpoint in [self, terminal] { - if let Self::RecoveryDisposition { - content_sha256, - identity_sha256, - copy_manifest_sha256, - copy_manifest_count, - created_at_unix_nanos, - revision, - state, - owner_fence_sha256, - owner_lease_acquired_at_unix_nanos, - owner_lease_expires_at_unix_nanos, - confirmed_absent_sha256, - retain_until_unix_nanos, - } = checkpoint - && !recovery_disposition_checkpoint_is_valid( - content_sha256, - identity_sha256, - copy_manifest_sha256, - *copy_manifest_count, - *created_at_unix_nanos, - *revision, - state.clone(), - owner_fence_sha256.as_deref(), - *owner_lease_acquired_at_unix_nanos, - *owner_lease_expires_at_unix_nanos, - confirmed_absent_sha256, - *retain_until_unix_nanos, - ) - { + if !recovery_disposition_checkpoint_is_valid(checkpoint) { return false; } } @@ -994,42 +937,52 @@ impl DurableIlmRecordCheckpoint { } } -fn recovery_disposition_checkpoint_is_valid( - content_sha256: &str, - identity_sha256: &str, - copy_manifest_sha256: &str, - copy_manifest_count: usize, - created_at_unix_nanos: i64, - revision: u64, - state: recovery_disposition::IlmRecoveryDispositionState, - owner_fence_sha256: Option<&str>, - owner_lease_acquired_at_unix_nanos: Option, - owner_lease_expires_at_unix_nanos: Option, - confirmed_absent_sha256: &[String], - retain_until_unix_nanos: i64, -) -> bool { +fn recovery_disposition_checkpoint_is_valid(checkpoint: &DurableIlmRecordCheckpoint) -> bool { use recovery_disposition::IlmRecoveryDispositionState::{Applying, Completed, Prepared}; + let DurableIlmRecordCheckpoint::RecoveryDisposition { + content_sha256, + identity_sha256, + copy_manifest_sha256, + copy_manifest_count, + created_at_unix_nanos, + revision, + state, + owner_fence_sha256, + owner_lease_acquired_at_unix_nanos, + owner_lease_expires_at_unix_nanos, + confirmed_absent_sha256, + retain_until_unix_nanos, + } = checkpoint + else { + return true; + }; + let owner_fence_sha256 = owner_fence_sha256.as_deref(); + is_canonical_sha256(content_sha256) && is_canonical_sha256(identity_sha256) && is_canonical_sha256(copy_manifest_sha256) - && copy_manifest_count > 0 - && created_at_unix_nanos > 0 - && revision > 0 - && retain_until_unix_nanos > 0 + && *copy_manifest_count > 0 + && *created_at_unix_nanos > 0 + && *revision > 0 + && *retain_until_unix_nanos > 0 && owner_fence_sha256.is_none_or(is_canonical_sha256) - && match (owner_fence_sha256, owner_lease_acquired_at_unix_nanos, owner_lease_expires_at_unix_nanos) { + && match ( + owner_fence_sha256, + *owner_lease_acquired_at_unix_nanos, + *owner_lease_expires_at_unix_nanos, + ) { (None, None, None) => true, (Some(_), Some(acquired), Some(expires)) => acquired > 0 && expires > acquired, _ => false, } - && confirmed_absent_sha256.len() <= copy_manifest_count + && confirmed_absent_sha256.len() <= *copy_manifest_count && confirmed_absent_sha256.iter().all(|digest| is_canonical_sha256(digest)) && confirmed_absent_sha256.windows(2).all(|pair| pair[0] < pair[1]) - && match state { + && match *state { Prepared => confirmed_absent_sha256.is_empty(), Applying => owner_fence_sha256.is_some(), - Completed => owner_fence_sha256.is_none() && confirmed_absent_sha256.len() == copy_manifest_count, + Completed => owner_fence_sha256.is_none() && confirmed_absent_sha256.len() == *copy_manifest_count, } } diff --git a/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs b/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs index 375b72a68..1ee698a6e 100644 --- a/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs +++ b/crates/ecstore/src/bucket/lifecycle/recovery_disposition.rs @@ -412,12 +412,8 @@ impl IlmRecoveryDisposition { }; let minimum_distance = match (previous.state, self.state) { (IlmRecoveryDispositionState::Prepared, IlmRecoveryDispositionState::Prepared) => { - if previous.owner.is_none() && self.owner.is_some() { - 1 - } else if previous.owner.is_some() - && self.owner.is_some() - && previous.owner != self.owner - && owner_change_is_fenced() + if self.owner.is_some() + && (previous.owner.is_none() || (previous.owner != self.owner && owner_change_is_fenced())) { 1 } else { diff --git a/rustfs/src/admin/handlers/ilm_transition.rs b/rustfs/src/admin/handlers/ilm_transition.rs index de5c28693..d511470d0 100644 --- a/rustfs/src/admin/handlers/ilm_transition.rs +++ b/rustfs/src/admin/handlers/ilm_transition.rs @@ -2002,12 +2002,11 @@ mod tests { execute_json.replace(export_sha256.as_str(), "too-short"), execute_json.replace("opaque-execute", ""), ] { - match parse_recovery_record_mutation_request(invalid.as_bytes()) { - Ok(request) => assert!( + if let Ok(request) = parse_recovery_record_mutation_request(invalid.as_bytes()) { + assert!( validate_recovery_record_mutation_request(&request).is_err(), "request should fail closed: {invalid}" - ), - Err(_) => {} + ); } }