fix(replication): harden live delete admission (#5599)

crates/replication/src/config.rs

@@ -18,9 +18,12 @@ use crate::rule::ReplicationRuleExt as _;
 use s3s::dto::DeleteMarkerReplicationStatus;
 use s3s::dto::DeleteReplicationStatus;
 use s3s::dto::Destination;
-use s3s::dto::{ExistingObjectReplicationStatus, ReplicationConfiguration, ReplicationRuleStatus, ReplicationRules};
+use s3s::dto::{
+    ExistingObjectReplicationStatus, ReplicaModificationsStatus, ReplicationConfiguration, ReplicationRule,
+    ReplicationRuleStatus, ReplicationRules,
+};
 use serde::{Deserialize, Serialize};
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use uuid::Uuid;
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
@@ -43,6 +46,44 @@ pub trait ReplicationConfigurationExt {
     fn get_destination(&self) -> Destination;
     fn has_active_rules(&self, prefix: &str, recursive: bool) -> bool;
     fn filter_target_arns(&self, obj: &ObjectOpts) -> Vec<String>;
+    fn filter_target_replication_decisions(&self, obj: &ObjectOpts) -> Vec<(String, bool)> {
+        self.filter_target_arns(obj)
+            .into_iter()
+            .map(|arn| {
+                let mut target = obj.clone();
+                target.target_arn = arn.clone();
+                (arn, self.replicate(&target))
+            })
+            .collect()
+    }
+}
+
+fn rule_replicates(rule: &ReplicationRule, obj: &ObjectOpts) -> bool {
+    if let Some(status) = &rule.existing_object_replication
+        && obj.existing_object
+        && status.status == ExistingObjectReplicationStatus::from_static(ExistingObjectReplicationStatus::DISABLED)
+    {
+        return false;
+    }
+
+    if obj.op_type != ReplicationType::Delete {
+        return rule.metadata_replicate(obj);
+    }
+
+    if !rule.metadata_replicate(obj) {
+        return false;
+    }
+
+    let version_purge = obj.version_id.is_some();
+    if version_purge {
+        rule.delete_replication
+            .as_ref()
+            .is_some_and(|delete| delete.status == DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED))
+    } else {
+        rule.delete_marker_replication.as_ref().is_some_and(|delete_marker| {
+            delete_marker.status == Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::ENABLED))
+        })
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -73,6 +114,58 @@ pub fn unsupported_replication_config_field(config: &ReplicationConfiguration) -
     None
 }
 
+pub fn invalid_replication_config_status_field(config: &ReplicationConfiguration) -> Option<&'static str> {
+    for rule in &config.rules {
+        if !matches!(rule.status.as_str(), ReplicationRuleStatus::ENABLED | ReplicationRuleStatus::DISABLED) {
+            return Some("Rule.Status");
+        }
+        if rule.existing_object_replication.as_ref().is_some_and(|existing| {
+            !matches!(
+                existing.status.as_str(),
+                ExistingObjectReplicationStatus::ENABLED | ExistingObjectReplicationStatus::DISABLED
+            )
+        }) {
+            return Some("Rule.ExistingObjectReplication.Status");
+        }
+        if rule.delete_replication.as_ref().is_some_and(|delete| {
+            !matches!(
+                delete.status.as_str(),
+                DeleteReplicationStatus::ENABLED | DeleteReplicationStatus::DISABLED
+            )
+        }) {
+            return Some("Rule.DeleteReplication.Status");
+        }
+        if rule
+            .delete_marker_replication
+            .as_ref()
+            .and_then(|delete| delete.status.as_ref())
+            .is_some_and(|status| {
+                !matches!(
+                    status.as_str(),
+                    DeleteMarkerReplicationStatus::ENABLED | DeleteMarkerReplicationStatus::DISABLED
+                )
+            })
+        {
+            return Some("Rule.DeleteMarkerReplication.Status");
+        }
+        if rule
+            .source_selection_criteria
+            .as_ref()
+            .and_then(|criteria| criteria.replica_modifications.as_ref())
+            .is_some_and(|modifications| {
+                !matches!(
+                    modifications.status.as_str(),
+                    ReplicaModificationsStatus::ENABLED | ReplicaModificationsStatus::DISABLED
+                )
+            })
+        {
+            return Some("Rule.SourceSelectionCriteria.ReplicaModifications.Status");
+        }
+    }
+
+    None
+}
+
 pub fn active_replication_rule_destination_arns(config: &ReplicationConfiguration) -> HashSet<String> {
     let mut arns = HashSet::new();
 
@@ -220,40 +313,7 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
     /// Determine whether an object should be replicated
     fn replicate(&self, obj: &ObjectOpts) -> bool {
         let rules = self.filter_actionable_rules(obj);
-
-        for rule in rules.iter() {
-            if rule.status == ReplicationRuleStatus::from_static(ReplicationRuleStatus::DISABLED) {
-                continue;
-            }
-
-            if let Some(status) = &rule.existing_object_replication
-                && obj.existing_object
-                && status.status == ExistingObjectReplicationStatus::from_static(ExistingObjectReplicationStatus::DISABLED)
-            {
-                return false;
-            }
-
-            if obj.op_type == ReplicationType::Delete {
-                if !rule.metadata_replicate(obj) {
-                    return false;
-                }
-
-                if obj.version_id.is_some() {
-                    return rule
-                        .delete_replication
-                        .clone()
-                        .is_some_and(|d| d.status == DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED));
-                } else {
-                    return rule.delete_marker_replication.clone().is_some_and(|d| {
-                        d.status == Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::ENABLED))
-                    });
-                }
-            }
-
-            // Regular object/metadata replication
-            return rule.metadata_replicate(obj);
-        }
-        false
+        rules.first().is_some_and(|rule| rule_replicates(rule, obj))
     }
 
     /// Check for an active rule
@@ -317,6 +377,41 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
         }
         arns
     }
+
+    fn filter_target_replication_decisions(&self, obj: &ObjectOpts) -> Vec<(String, bool)> {
+        let rules = self.filter_actionable_rules(obj);
+        let role = self.role.trim();
+        if !role.is_empty() {
+            let mut selected = None;
+            for rule in &rules {
+                if selected.is_none_or(|current: &ReplicationRule| rule.priority > current.priority) {
+                    selected = Some(rule);
+                }
+            }
+            return vec![(role.to_string(), selected.is_some_and(|rule| rule_replicates(rule, obj)))];
+        }
+
+        let mut target_indexes: HashMap<&str, usize> = HashMap::new();
+        let mut selected_rules: Vec<(&str, &ReplicationRule)> = Vec::new();
+        for rule in &rules {
+            let arn = rule.destination.bucket.trim();
+            if arn.is_empty() {
+                continue;
+            }
+            if let Some(index) = target_indexes.get(arn).copied() {
+                if rule.priority > selected_rules[index].1.priority {
+                    selected_rules[index].1 = rule;
+                }
+            } else {
+                target_indexes.insert(arn, selected_rules.len());
+                selected_rules.push((arn, rule));
+            }
+        }
+        selected_rules
+            .into_iter()
+            .map(|(arn, rule)| (arn.to_string(), rule_replicates(rule, obj)))
+            .collect()
+    }
 }
 
 #[cfg(test)]
@@ -324,8 +419,8 @@ mod tests {
     use super::*;
     use s3s::dto::{
         DeleteMarkerReplication, DeleteReplication, Destination, EncryptionConfiguration, ExistingObjectReplication, Metrics,
-        MetricsStatus, ReplicationRule, ReplicationTime, ReplicationTimeStatus, ReplicationTimeValue, SourceSelectionCriteria,
-        SseKmsEncryptedObjects, SseKmsEncryptedObjectsStatus,
+        MetricsStatus, ReplicaModifications, ReplicationRule, ReplicationTime, ReplicationTimeStatus, ReplicationTimeValue,
+        SourceSelectionCriteria, SseKmsEncryptedObjects, SseKmsEncryptedObjectsStatus,
     };
 
     fn replication_rule(id: &str, arn: &str) -> ReplicationRule {
@@ -627,61 +722,75 @@ mod tests {
         );
     }
 
+    #[test]
+    fn role_delete_decision_follows_highest_priority_rule() {
+        let role = "arn:rustfs:replication:us-east-1:role-target:bucket";
+        let destination = "arn:rustfs:replication:us-east-1:target:bucket";
+        let config = ReplicationConfiguration {
+            role: role.to_string(),
+            rules: vec![
+                delete_marker_rule("low-priority-enabled", destination, "logs/", 1, true),
+                delete_marker_rule("high-priority-disabled", destination, "logs/2026/", 5, false),
+            ],
+        };
+        let opts = ObjectOpts {
+            name: "logs/2026/app.log".to_string(),
+            op_type: ReplicationType::Delete,
+            delete_marker: true,
+            ..Default::default()
+        };
+
+        assert_eq!(
+            config.filter_target_replication_decisions(&opts),
+            vec![(role.to_string(), false)],
+            "the role target must use the highest-priority matching rule"
+        );
+    }
+
     #[test]
     fn version_purge_uses_delete_replication_for_object_and_marker_versions() {
         let arn = "arn:rustfs:replication:us-east-1:target:bucket";
-        let mut rule = replication_rule("delete", arn);
-        rule.delete_marker_replication = Some(DeleteMarkerReplication {
-            status: Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::DISABLED)),
-        });
+        let mut rule = delete_marker_rule("delete-switches", arn, "", 1, true);
         rule.delete_replication = Some(DeleteReplication {
-            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
+            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::DISABLED),
         });
         let mut config = ReplicationConfiguration {
             role: String::new(),
             rules: vec![rule],
         };
-        let version_id = Some(Uuid::new_v4());
 
-        for delete_marker in [false, true] {
-            assert!(config.replicate(&ObjectOpts {
-                name: "object".to_string(),
-                version_id,
-                delete_marker,
-                op_type: ReplicationType::Delete,
-                ..Default::default()
-            }));
+        for version_id in [Some(Uuid::new_v4()), Some(Uuid::nil())] {
+            for delete_marker in [false, true] {
+                assert!(!config.replicate(&ObjectOpts {
+                    name: "object".to_string(),
+                    op_type: ReplicationType::Delete,
+                    version_id,
+                    delete_marker,
+                    ..Default::default()
+                }));
+            }
         }
-        assert!(!config.replicate(&ObjectOpts {
+
+        let stored_marker = ObjectOpts {
             name: "object".to_string(),
-            delete_marker: true,
             op_type: ReplicationType::Delete,
+            delete_marker: true,
             ..Default::default()
-        }));
+        };
+        assert!(config.replicate(&stored_marker), "stored markers must use DeleteMarkerReplication");
+        assert_eq!(config.filter_target_replication_decisions(&stored_marker), vec![(arn.to_string(), true)]);
 
-        let rule = &mut config.rules[0];
-        rule.delete_marker_replication = Some(DeleteMarkerReplication {
-            status: Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::ENABLED)),
+        config.rules[0].delete_replication = Some(DeleteReplication {
+            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
         });
-        rule.delete_replication = Some(DeleteReplication {
-            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::DISABLED),
-        });
-
-        for delete_marker in [false, true] {
-            assert!(!config.replicate(&ObjectOpts {
-                name: "object".to_string(),
-                version_id,
-                delete_marker,
-                op_type: ReplicationType::Delete,
-                ..Default::default()
-            }));
-        }
         assert!(config.replicate(&ObjectOpts {
             name: "object".to_string(),
-            delete_marker: true,
             op_type: ReplicationType::Delete,
+            version_id: Some(Uuid::nil()),
+            delete_marker: true,
             ..Default::default()
         }));
+        assert!(config.replicate(&stored_marker));
     }
 
     #[test]
@@ -721,4 +830,77 @@ mod tests {
         });
         assert_eq!(unsupported_replication_config_field(&config), Some("Destination.ReplicationTime"));
     }
+
+    #[test]
+    fn invalid_replication_status_fields_are_reported_before_persistence() {
+        let arn = "arn:rustfs:replication:us-east-1:target:bucket";
+        let mut config = ReplicationConfiguration {
+            role: String::new(),
+            rules: vec![replication_rule("invalid-status", arn)],
+        };
+
+        config.rules[0].status = ReplicationRuleStatus::from_static("Invalid");
+        assert_eq!(invalid_replication_config_status_field(&config), Some("Rule.Status"));
+
+        config.rules[0] = replication_rule("invalid-status", arn);
+        config.rules[0].existing_object_replication = Some(ExistingObjectReplication {
+            status: ExistingObjectReplicationStatus::from_static("Invalid"),
+        });
+        assert_eq!(
+            invalid_replication_config_status_field(&config),
+            Some("Rule.ExistingObjectReplication.Status")
+        );
+
+        config.rules[0] = replication_rule("invalid-status", arn);
+        config.rules[0].delete_replication = Some(DeleteReplication {
+            status: DeleteReplicationStatus::from_static("Invalid"),
+        });
+        assert_eq!(invalid_replication_config_status_field(&config), Some("Rule.DeleteReplication.Status"));
+
+        config.rules[0] = replication_rule("invalid-status", arn);
+        config.rules[0].delete_marker_replication = Some(DeleteMarkerReplication {
+            status: Some(DeleteMarkerReplicationStatus::from_static("Invalid")),
+        });
+        assert_eq!(
+            invalid_replication_config_status_field(&config),
+            Some("Rule.DeleteMarkerReplication.Status")
+        );
+
+        config.rules[0] = replication_rule("invalid-status", arn);
+        config.rules[0].source_selection_criteria = Some(SourceSelectionCriteria {
+            replica_modifications: Some(ReplicaModifications {
+                status: ReplicaModificationsStatus::from_static("Invalid"),
+            }),
+            sse_kms_encrypted_objects: None,
+        });
+        assert_eq!(
+            invalid_replication_config_status_field(&config),
+            Some("Rule.SourceSelectionCriteria.ReplicaModifications.Status")
+        );
+    }
+
+    #[test]
+    fn target_decisions_choose_highest_priority_rule_per_destination() {
+        let target_a = "arn:rustfs:replication:us-east-1:target:a";
+        let target_b = "arn:rustfs:replication:us-east-1:target:b";
+        let mut a_low = delete_marker_rule("a-low", target_a, "logs/", 1, true);
+        let b = delete_marker_rule("b", target_b, "logs/", 2, true);
+        let a_high = delete_marker_rule("a-high", target_a, "logs/2026/", 5, false);
+        a_low.delete_replication = Some(DeleteReplication {
+            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
+        });
+        let config = ReplicationConfiguration {
+            role: String::new(),
+            rules: vec![a_low, b, a_high],
+        };
+
+        let decisions = config.filter_target_replication_decisions(&ObjectOpts {
+            name: "logs/2026/app.log".to_string(),
+            op_type: ReplicationType::Delete,
+            delete_marker: true,
+            ..Default::default()
+        });
+
+        assert_eq!(decisions, vec![(target_a.to_string(), false), (target_b.to_string(), true)]);
+    }
 }

crates/replication/src/lib.rs

@@ -30,8 +30,8 @@ pub mod tagging;
 
 pub use config::{
     ObjectOpts, ReplicationConfigurationExt, ReplicationTargetValidationError, active_replication_rule_destination_arns,
-    replication_target_arns, should_remove_replication_target, unsupported_replication_config_field,
-    validate_replication_config_target_arns,
+    invalid_replication_config_status_field, replication_target_arns, should_remove_replication_target,
+    unsupported_replication_config_field, validate_replication_config_target_arns,
 };
 pub use delete::{
     DeletedObjectReplicationInfo, is_retryable_delete_replication_head_error, is_version_delete_replication,

crates/replication/src/operation.rs

@@ -133,16 +133,13 @@ pub fn delete_replication_state_from_config(
         replica: source.replica,
         ..Default::default()
     };
-    let target_arns = config.filter_target_arns(&opts);
-    if target_arns.is_empty() {
+    let target_decisions = config.filter_target_replication_decisions(&opts);
+    if target_decisions.is_empty() {
         return None;
     }
 
     let mut decision = ReplicateDecision::new();
-    for target_arn in target_arns {
-        let mut target_opts = opts.clone();
-        target_opts.target_arn = target_arn.clone();
-        let replicate = config.replicate(&target_opts);
+    for (target_arn, replicate) in target_decisions {
         decision.set(ReplicateTargetDecision::new(target_arn, replicate, false));
     }
     if !decision.replicate_any() {
@@ -174,20 +171,13 @@ pub struct ReplicationDeleteScheduleInput<'a> {
     pub deleted_delete_marker_version: bool,
 }
 
-fn delete_version_purge_source_status(status: &ReplicationStatusType) -> bool {
-    status == &ReplicationStatusType::Replica
-        || status == &ReplicationStatusType::Pending
-        || status == &ReplicationStatusType::Completed
-        || status == &ReplicationStatusType::Failed
-}
-
 pub fn should_schedule_delete_replication(input: ReplicationDeleteScheduleInput<'_>) -> bool {
     if input.replication_request {
         return false;
     }
 
-    if input.version_id_requested && !input.deleted_delete_marker_version && !input.source_delete_marker {
-        return delete_version_purge_source_status(input.source_replication_status);
+    if input.version_id_requested {
+        return input.source_version_purge_status == &VersionPurgeStatusType::Pending;
     }
 
     input.source_replication_status == &ReplicationStatusType::Replica
@@ -433,9 +423,7 @@ mod tests {
             delete_marker_replication: Some(DeleteMarkerReplication {
                 status: Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::ENABLED)),
             }),
-            delete_replication: Some(DeleteReplication {
-                status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
-            }),
+            delete_replication: None,
             destination: Destination {
                 bucket: arn.to_string(),
                 ..Default::default()
@@ -500,11 +488,15 @@ mod tests {
     }
 
     #[test]
-    fn delete_replication_state_tracks_delete_marker_version_purges() {
+    fn delete_replication_state_uses_delete_switch_for_marker_version_purges() {
         let arn = "arn:aws:s3:::target-bucket";
-        let config = ReplicationConfiguration {
+        let mut rule = delete_replication_rule(arn, false);
+        rule.delete_replication = Some(DeleteReplication {
+            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::DISABLED),
+        });
+        let mut config = ReplicationConfiguration {
             role: arn.to_string(),
-            rules: vec![delete_replication_rule(arn, false)],
+            rules: vec![rule],
         };
         let source = ReplicationDeleteStateSource {
             name: "test/object.txt".to_string(),
@@ -514,8 +506,16 @@ mod tests {
             replica: false,
         };
 
+        assert!(
+            delete_replication_state_from_config(&config, &source).is_none(),
+            "delete-marker version purge must not use the enabled marker-creation switch"
+        );
+
+        config.rules[0].delete_replication = Some(DeleteReplication {
+            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
+        });
         let state = delete_replication_state_from_config(&config, &source)
-            .expect("delete-marker version purge should honor delete replication rules");
+            .expect("delete-marker version purge should honor the enabled permanent-delete switch");
         let pending = format!("{arn}=PENDING;");
 
         assert_eq!(state.version_purge_status_internal.as_deref(), Some(pending.as_str()));
@@ -536,8 +536,8 @@ mod tests {
     }
 
     #[test]
-    fn delete_replication_schedule_keeps_marker_and_version_purges() {
-        assert!(should_schedule_delete_replication(ReplicationDeleteScheduleInput {
+    fn delete_replication_schedule_requires_current_admission_for_version_purges() {
+        assert!(!should_schedule_delete_replication(ReplicationDeleteScheduleInput {
             replication_request: false,
             version_id_requested: true,
             source_delete_marker: true,
@@ -549,8 +549,16 @@ mod tests {
             replication_request: false,
             version_id_requested: true,
             source_delete_marker: false,
-            source_replication_status: &ReplicationStatusType::Completed,
-            source_version_purge_status: &VersionPurgeStatusType::Empty,
+            source_replication_status: &ReplicationStatusType::Empty,
+            source_version_purge_status: &VersionPurgeStatusType::Pending,
+            deleted_delete_marker_version: true,
+        }));
+        assert!(should_schedule_delete_replication(ReplicationDeleteScheduleInput {
+            replication_request: false,
+            version_id_requested: true,
+            source_delete_marker: false,
+            source_replication_status: &ReplicationStatusType::Empty,
+            source_version_purge_status: &VersionPurgeStatusType::Pending,
             deleted_delete_marker_version: false,
         }));
         assert!(should_schedule_delete_replication(ReplicationDeleteScheduleInput {

crates/replication/src/queue.rs

@@ -410,6 +410,21 @@ mod tests {
         assert_eq!(delete_info.delete_object.delete_marker_version_id, None);
     }
 
+    #[test]
+    fn heal_queue_action_preserves_pending_null_version_purge() {
+        let mut roi = replicate_object_info(ReplicationStatusType::Completed);
+        roi.version_id = Some(Uuid::nil());
+        roi.version_purge_status = VersionPurgeStatusType::Pending;
+
+        let action = replication_heal_queue_action(&mut roi);
+
+        let ReplicationHealQueueAction::QueueDelete(delete_info) = action else {
+            panic!("expected null-version purge delete queue action");
+        };
+        assert_eq!(delete_info.delete_object.version_id, Some(Uuid::nil()));
+        assert_eq!(delete_info.delete_object.delete_marker_version_id, None);
+    }
+
     #[test]
     fn replication_priority_parses_known_values_and_defaults_unknown() {
         assert_eq!(ReplicationPriority::from_str("fast"), Ok(ReplicationPriority::Fast));