diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 5dbff736e..9d2eb8c52 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -2331,6 +2331,8 @@ fn append_bootstrap_bucket_items( bucket.expiry_lc_config_updated_at, |item, value| { item.expiry_lc_config = Some(value); + // `updated_at` here is the entry's expiry axis (see the + // SRBucketInfo construction), not the wall clock. item.expiry_updated_at = item.updated_at; Ok(()) }, @@ -4343,7 +4345,14 @@ async fn build_sr_info(state: &SiteReplicationState, local_peer: &PeerInfo) -> S entry.versioning_config_updated_at = maybe_time(metadata.versioning_config_updated_at); entry.replication_config_updated_at = maybe_time(metadata.replication_config_updated_at); entry.quota_config_updated_at = maybe_time(metadata.quota_config_updated_at); - entry.expiry_lc_config_updated_at = maybe_time(metadata.lifecycle_config_updated_at); + // The expiry axis, not the whole-config write time: local + // transition-only edits inflate the latter, and a repair item + // stamped with it could out-rank a newer real expiry edit on a + // third site. Legacy configs without the axis fall back to the + // whole-config time (bounded staleness, same as the receiver). + entry.expiry_lc_config_updated_at = lifecycle_expiry_updated_at(&metadata.lifecycle_config_xml) + .map(Some) + .unwrap_or_else(|| maybe_time(metadata.lifecycle_config_updated_at)); entry.cors_config_updated_at = maybe_time(metadata.cors_config_updated_at); entry.replication_targets_online = Some(site_replication_targets_online(&bucket.name, &metadata.replication_config_xml).await); @@ -6865,23 +6874,22 @@ fn merge_incoming_replication_config( /// Merge a peer's ILM expiry document into the local lifecycle config. /// /// Mirrors MinIO's `mergeWithCurrentLCConfig` with one hardening: incoming -/// transition fields are discarded outright (trust boundary — a peer must -/// never install tier/transition rules on this site, whatever it sends). -/// Local transition rules always survive; a delete (`incoming == None`) -/// therefore merges with the empty set instead of dropping the whole config. +/// site-local fields (transitions, abort-multipart, del-marker expiration — +/// exactly what MinIO's `CloneNonTransition` sender never emits) are +/// discarded outright at the trust boundary, whatever the peer sends. Local +/// site-local fields always survive; a delete (`incoming == None`) therefore +/// merges with the empty set instead of dropping the whole config. fn merge_incoming_lifecycle_config( incoming: Option, local: Option, updated_at: Option, ) -> Option { - // Incoming rules reduced to their expiry side (trust boundary). Rules - // that carry no expiry semantics after the strip — transition-only or - // abort-multipart-only — are site-local and are not installed. + // Incoming rules reduced to their traveling expiry side. Rules with no + // expiry semantics after the strip are not installed. let mut incoming_by_id: HashMap = HashMap::new(); let mut incoming_order: Vec = Vec::new(); for mut rule in incoming.into_iter().flat_map(|config| config.rules) { - rule.transitions = None; - rule.noncurrent_version_transitions = None; + strip_site_local_lifecycle_fields(&mut rule); if !lifecycle_rule_has_expiry(&rule) { continue; } @@ -6900,23 +6908,28 @@ fn merge_incoming_lifecycle_config( let id = rule.id.clone().unwrap_or_default(); if let Some(mut incoming_rule) = incoming_by_id.remove(&id) { incoming_order.retain(|pending| pending != &id); - // The incoming expiry side wins; the local transition side is + // The incoming expiry side wins; the local site-local side is // authoritative (MinIO CloneNonTransition + restore). incoming_rule.transitions = rule.transitions.take(); incoming_rule.noncurrent_version_transitions = rule.noncurrent_version_transitions.take(); + incoming_rule.abort_incomplete_multipart_upload = rule.abort_incomplete_multipart_upload.take(); + incoming_rule.del_marker_expiration = rule.del_marker_expiration.take(); rules.push(incoming_rule); } else if lifecycle_rule_has_expiry(&rule) { - // Expiry rule dropped upstream: remove a pure-expiry rule, or - // strip only the expiry side when a transition side remains. - if lifecycle_rule_has_transition(&rule) { - rule.expiration = None; - rule.noncurrent_version_expiration = None; - rule.del_marker_expiration = None; + // Expiry rule dropped upstream: strip only the traveling expiry + // side; the rule survives while any site-local action remains. + rule.expiration = None; + rule.noncurrent_version_expiration = None; + if lifecycle_rule_has_transition(&rule) + || rule.abort_incomplete_multipart_upload.is_some() + || rule.del_marker_expiration.is_some() + { rules.push(rule); } } else { - // No expiry semantics (transition-only / abort-mpu-only): not - // managed by expiry replication, keep untouched. + // No traveling expiry semantics (transition-only / abort-mpu-only + // / del-marker-only): not managed by expiry replication, keep + // untouched. rules.push(rule); } } @@ -6932,17 +6945,20 @@ fn merge_incoming_lifecycle_config( Some(s3s::dto::BucketLifecycleConfiguration { rules, - // Stamp the expiry axis so the staleness guard compares expiry - // updates against expiry updates; a local transition-only edit moves - // only the whole-config timestamp, not this one. + // Record the expiry axis the staleness guard compares on. (The PUT + // path stamps `expiry_updated_at` only when the expiry subset + // changes, so this axis is not inflated by transition-only edits.) expiry_updated_at: updated_at.map(s3s::dto::Timestamp::from).or(local_expiry_updated_at), }) } -/// True when the rule carries expiry semantics that `replicateILMExpiry` is -/// allowed to propagate. +/// True when the rule carries the expiry semantics that `replicateILMExpiry` +/// propagates. Del-marker expiration and abort-multipart are deliberately +/// excluded: MinIO's sender never emits them (`CloneNonTransition` drops +/// both), so treating them as traveling state would let a MinIO peer's +/// broadcast delete this site's del-marker-only rules. fn lifecycle_rule_has_expiry(rule: &s3s::dto::LifecycleRule) -> bool { - rule.expiration.is_some() || rule.noncurrent_version_expiration.is_some() || rule.del_marker_expiration.is_some() + rule.expiration.is_some() || rule.noncurrent_version_expiration.is_some() } fn lifecycle_rule_has_transition(rule: &s3s::dto::LifecycleRule) -> bool { @@ -6953,6 +6969,15 @@ fn lifecycle_rule_has_transition(rule: &s3s::dto::LifecycleRule) -> bool { .is_some_and(|transitions| !transitions.is_empty()) } +/// Remove the fields that never travel between sites (MinIO +/// `CloneNonTransition` parity). +fn strip_site_local_lifecycle_fields(rule: &mut s3s::dto::LifecycleRule) { + rule.transitions = None; + rule.noncurrent_version_transitions = None; + rule.abort_incomplete_multipart_upload = None; + rule.del_marker_expiration = None; +} + /// Reduce a lifecycle XML document to the expiry subset that is allowed to /// travel between sites (what MinIO's sender emits): transition fields are /// stripped and rules left with no expiry semantics are dropped. Returns @@ -6977,8 +7002,7 @@ fn lifecycle_expiry_subset_xml(raw: &[u8]) -> Option> { .rules .into_iter() .filter_map(|mut rule| { - rule.transitions = None; - rule.noncurrent_version_transitions = None; + strip_site_local_lifecycle_fields(&mut rule); lifecycle_rule_has_expiry(&rule).then_some(rule) }) .collect(); @@ -6998,6 +7022,29 @@ fn lifecycle_expiry_subset_xml(raw: &[u8]) -> Option> { } } +/// The expiry replication axis persisted in a lifecycle XML document, if any. +/// Used for the SRInfo bucket entry so bootstrap/repair items carry the +/// expiry axis instead of the whole-config write time (which local +/// transition-only edits inflate). +fn lifecycle_expiry_updated_at(raw: &[u8]) -> Option { + if raw.is_empty() { + return None; + } + deserialize::(raw) + .ok() + .and_then(|config| config.expiry_updated_at) + .map(OffsetDateTime::from) +} + +/// MinIO emits a zero-rule `…` +/// document for its delete tombstone / transition-only states; the s3s +/// deserializer rejects a missing `` outright. Such a document is the +/// "no expiry rules here" statement and must map to delete semantics rather +/// than an error. +fn lifecycle_document_has_rules(raw: &[u8]) -> bool { + raw.windows(b" Option { if let Some(rule_id) = rule.id.as_deref() { if let Some(deployment_id) = rule_id.strip_prefix("site-repl-") @@ -8130,32 +8177,49 @@ async fn apply_bucket_meta_item(item: SRBucketMeta) -> S3Result<()> { // Receiver-side gate, symmetric with the sender hook: a peer must not // install expiry rules here while `replicateILMExpiry` is off. When // the state cannot be read, fall through and apply (pre-gate - // behavior) rather than silently dropping a legitimate update. + // behavior) rather than silently dropping a legitimate update. Note + // the gate acks with 200 — the sender treats the item as delivered + // and will not retry; items skipped inside the enable-flag + // propagation window are healed by repair, not by retry. if let Ok(state) = load_site_replication_state().await && !site_replication_state_replicates_ilm_expiry(&state) { return Ok(()); } - let incoming = item - .expiry_lc_config - .as_ref() - .map(|raw| { + let incoming = match item.expiry_lc_config.as_ref() { + Some(raw) => { let data = decode_bucket_meta_wire_value(raw); - deserialize::(&data) - }) - .transpose() - .map_err(|e| s3_error!(InvalidRequest, "invalid lifecycle config: {e}"))?; + match deserialize::(&data) { + Ok(config) => Some(config), + // MinIO's delete tombstone / transition-only state is a + // zero-rule document the strict deserializer rejects; it + // means "no expiry rules here" (delete semantics). + Err(_) if !lifecycle_document_has_rules(&data) => None, + Err(e) => return Err(s3_error!(InvalidRequest, "invalid lifecycle config: {e}")), + } + } + None => None, + }; let local = match metadata_sys::get_lifecycle_config(&item.bucket).await { Ok((config, _)) => Some(config), Err(StorageError::ConfigNotFound) => None, Err(err) => return Err(ApiError::from(err).into()), }; + // Staleness axis: the config's expiry_updated_at when present. When + // the config is absent (deleted) or predates the axis field, fall + // back to the whole-config write time — it survives deletion in + // bucket metadata and is the deletion's lower bound, so a delayed + // stale broadcast cannot resurrect deleted expiry rules. + let whole_config_axis = metadata_sys::get(&item.bucket) + .await + .map(|bucket_meta| bucket_meta.lifecycle_config_updated_at) + .unwrap_or(OffsetDateTime::UNIX_EPOCH); let local_expiry_updated_at = local .as_ref() .and_then(|config| config.expiry_updated_at.clone()) .map(OffsetDateTime::from) - .unwrap_or(OffsetDateTime::UNIX_EPOCH); + .unwrap_or(whole_config_axis); if is_stale_update(local_expiry_updated_at, incoming_updated_at) { return Ok(()); } @@ -14725,6 +14789,72 @@ mod tests { assert_eq!(OffsetDateTime::from(stamped).unix_timestamp(), updated_at.unix_timestamp()); } + /// MinIO's sender never emits del-marker-expiration rules + /// (CloneNonTransition drops them), so a MinIO expiry broadcast must not + /// delete this site's del-marker-only rules, and the local del-marker + /// side of a same-id rule is authoritative. + #[test] + fn test_merge_incoming_lifecycle_del_marker_rules_stay_local() { + let del_marker_only = |id: &str| { + let mut rule = lc_rule(id, None, None); + rule.del_marker_expiration = Some(s3s::dto::DelMarkerExpiration { days: Some(3) }); + rule + }; + + // A local del-marker-only rule survives an incoming expiry document + // that does not mention it. + let merged = merge_incoming_lifecycle_config( + Some(lc_config(vec![lc_rule("e1", Some(7), None)])), + Some(lc_config(vec![del_marker_only("dm-local")])), + None, + ) + .expect("merge should keep rules"); + let mut ids = rule_ids(&merged); + ids.sort_unstable(); + assert_eq!(ids, vec!["dm-local", "e1"]); + + // Same-id: the incoming expiry side wins, the local del-marker / + // abort-mpu side is authoritative and an incoming del-marker field is + // discarded at the trust boundary. + let mut local_mixed = lc_rule("r1", Some(1), None); + local_mixed.del_marker_expiration = Some(s3s::dto::DelMarkerExpiration { days: Some(3) }); + local_mixed.abort_incomplete_multipart_upload = Some(s3s::dto::AbortIncompleteMultipartUpload { + days_after_initiation: Some(5), + }); + let mut incoming_mixed = lc_rule("r1", Some(7), None); + incoming_mixed.del_marker_expiration = Some(s3s::dto::DelMarkerExpiration { days: Some(9) }); + + let merged = + merge_incoming_lifecycle_config(Some(lc_config(vec![incoming_mixed])), Some(lc_config(vec![local_mixed])), None) + .expect("merge should keep rules"); + let r1 = &merged.rules[0]; + assert_eq!(r1.expiration.as_ref().and_then(|e| e.days), Some(7)); + assert_eq!(r1.del_marker_expiration.as_ref().and_then(|d| d.days), Some(3), "local del-marker wins"); + assert_eq!( + r1.abort_incomplete_multipart_upload + .as_ref() + .and_then(|a| a.days_after_initiation), + Some(5), + "local abort-mpu wins" + ); + } + + /// MinIO emits a zero-rule lifecycle document (delete tombstone / + /// transition-only state); the strict deserializer rejects it, so the + /// apply path must recognize it as delete semantics via the rule sniff. + #[test] + fn test_lifecycle_document_has_rules_sniff() { + assert!(!lifecycle_document_has_rules( + b"2026-01-01T00:00:00Z" + )); + assert!(lifecycle_document_has_rules( + b"x" + )); + assert!(lifecycle_document_has_rules( + b"" + )); + } + /// Sender-side filter: only the expiry subset leaves this site. MinIO /// peers install incoming rules verbatim, so a full document would plant /// this site's transition rules there. diff --git a/rustfs/src/app/bucket_usecase.rs b/rustfs/src/app/bucket_usecase.rs index fdc711a17..7d55035e9 100644 --- a/rustfs/src/app/bucket_usecase.rs +++ b/rustfs/src/app/bucket_usecase.rs @@ -1158,6 +1158,19 @@ fn lifecycle_has_expiry_rules(config: &BucketLifecycleConfiguration) -> bool { }) } +/// Status-independent presence of the expiry subset that site replication +/// propagates (`replicateILMExpiry`): expiration / noncurrent-version +/// expiration only. Distinct from [`lifecycle_has_expiry_rules`], which +/// filters on ENABLED for scanner scheduling — editing a Disabled expiry rule +/// must still advance the replication axis. Del-marker expiration and +/// abort-multipart are site-local and never travel. +fn lifecycle_rules_have_expiry(config: &BucketLifecycleConfiguration) -> bool { + config + .rules + .iter() + .any(|rule| rule.expiration.is_some() || rule.noncurrent_version_expiration.is_some()) +} + fn lifecycle_has_abort_multipart_rules(config: &BucketLifecycleConfiguration) -> bool { config.rules.iter().any(|rule| { rule.status == ExpirationStatus::from_static(ExpirationStatus::ENABLED) @@ -2186,7 +2199,24 @@ impl DefaultBucketUsecase { return Err(s3_error!(InvalidArgument, "{err}")); } - input_cfg.expiry_updated_at = Some(Timestamp::from(time::OffsetDateTime::now_utc())); + // Stamp the expiry axis only when the expiry subset can have changed + // (MinIO: HasExpiry() || expiryRuleRemoved). Site-replication peers + // judge lc-config staleness on this axis; a transition-only edit that + // advanced it would let this site's stale expiry subset shadow — and + // roll back — a newer peer expiry edit fleet-wide. + let previous_expiry_updated_at = match metadata_sys::get_lifecycle_config(&bucket).await { + Ok((previous, _)) => { + if lifecycle_rules_have_expiry(&input_cfg) || lifecycle_rules_have_expiry(&previous) { + Some(Timestamp::from(time::OffsetDateTime::now_utc())) + } else { + previous.expiry_updated_at + } + } + // No previous config (or unreadable): stamping is the + // conservative pre-existing behavior. + Err(_) => lifecycle_rules_have_expiry(&input_cfg).then(|| Timestamp::from(time::OffsetDateTime::now_utc())), + }; + input_cfg.expiry_updated_at = previous_expiry_updated_at; let data = serialize_config(&input_cfg)?; update_bucket_config_for_incarnation(&bucket, BUCKET_LIFECYCLE_CONFIG, data, expected_incarnation_id) .await @@ -2197,7 +2227,14 @@ impl DefaultBucketUsecase { let mut item = sr_bucket_meta_item(bucket.clone(), "lc-config"); item.expiry_lc_config = Some(serialize_config(&input_cfg).and_then(|bytes| String::from_utf8(bytes).map_err(to_internal_error))?); - item.expiry_updated_at = item.updated_at; + // The item travels with the expiry axis, not the wall clock: a site + // whose expiry knowledge is old (or absent — UNIX_EPOCH) must not + // out-rank newer peer expiry state at the receivers. + item.expiry_updated_at = input_cfg + .expiry_updated_at + .clone() + .map(time::OffsetDateTime::from) + .or(Some(time::OffsetDateTime::UNIX_EPOCH)); if let Err(err) = site_replication_bucket_meta_hook(item).await { warn!(bucket = %bucket, error = ?err, "site replication bucket lifecycle hook failed"); }