fix(replication): enforce bucket replication switches (#5449)

* fix(replication): enforce bucket replication switches

* fix(replication): satisfy delete admission clippy lint

* fix(replication): restore MinIO tag filter behavior

---------

Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
Co-authored-by: cxymds <cxymds@gmail.com>

crates/ecstore/src/api/mod.rs

@@ -188,8 +188,8 @@ pub mod bucket {
             get_global_replication_stats, init_background_replication, read_durable_mrf_backlog, replication_state_to_filemeta,
             replication_status_to_filemeta, replication_statuses_map, replication_target_arns, resync_start_conflict_id,
             should_remove_replication_target, should_schedule_delete_replication, should_use_existing_delete_replication_info,
-            should_use_existing_delete_replication_source, validate_replication_config_target_arns,
-            version_purge_status_to_filemeta,
+            should_use_existing_delete_replication_source, unsupported_replication_config_field,
+            validate_replication_config_target_arns, version_purge_status_to_filemeta,
         };
     }
 

crates/ecstore/src/bucket/replication/mod.rs

@@ -46,7 +46,7 @@ mod runtime_boundary;
 pub use datatypes::ResyncStatusType;
 pub use replication_config_boundary::{
     ObjectOpts, ReplicationConfigurationExt, ReplicationTargetValidationError, replication_target_arns,
-    should_remove_replication_target, validate_replication_config_target_arns,
+    should_remove_replication_target, unsupported_replication_config_field, validate_replication_config_target_arns,
 };
 #[cfg(test)]
 pub(crate) use replication_filemeta_boundary::ReplicateTargetDecision;

crates/ecstore/src/bucket/replication/replication_config_boundary.rs

@@ -14,5 +14,5 @@
 
 pub use rustfs_replication::{
     ObjectOpts, ReplicationConfigurationExt, ReplicationTargetValidationError, replication_target_arns,
-    should_remove_replication_target, validate_replication_config_target_arns,
+    should_remove_replication_target, unsupported_replication_config_field, validate_replication_config_target_arns,
 };

crates/ecstore/src/bucket/replication/replication_object_bridge.rs

@@ -14,8 +14,11 @@
 
 use std::{collections::HashMap, sync::Arc};
 
+use super::replication_error_boundary::Result;
 use super::replication_filemeta_boundary::{ReplicateDecision, ReplicationStatusType, ReplicationType};
-use super::replication_object_config::{check_replicate_delete, get_must_replicate_options, must_replicate};
+use super::replication_object_config::{
+    check_replicate_delete, check_replicate_delete_strict, get_must_replicate_options, must_replicate,
+};
 use super::replication_object_decision_boundary::MustReplicateOptions;
 use super::replication_pool::{schedule_replication, schedule_replication_delete};
 use super::replication_queue_boundary::DeletedObjectReplicationInfo;
@@ -50,6 +53,16 @@ impl ReplicationObjectBridge {
         check_replicate_delete(bucket, object, source, opts, get_error).await
     }
 
+    pub async fn check_delete_strict(
+        bucket: &str,
+        object: &ObjectToDelete,
+        source: &ObjectInfo,
+        opts: &ObjectOptions,
+        get_error: Option<String>,
+    ) -> Result<ReplicateDecision> {
+        check_replicate_delete_strict(bucket, object, source, opts, get_error).await
+    }
+
     pub async fn schedule_object<S: ReplicationStorage>(
         object: ObjectInfo,
         storage: Arc<S>,

crates/ecstore/src/bucket/replication/replication_object_config.rs

@@ -169,9 +169,8 @@ pub(crate) async fn check_replicate_delete(
     del_opts: &ObjectOptions,
     gerr: Option<String>,
 ) -> ReplicateDecision {
-    let rcfg = match get_replication_config(bucket).await {
-        Ok(Some(config)) => config,
-        Ok(None) => return ReplicateDecision::default(),
+    match check_replicate_delete_strict(bucket, dobj, oi, del_opts, gerr).await {
+        Ok(decision) => decision,
         Err(err) => {
             error!(
                 event = EVENT_RESYNC_CONFIG_LOOKUP_SKIPPED,
@@ -182,16 +181,30 @@ pub(crate) async fn check_replicate_delete(
                 error = %err,
                 "Failed to look up replication config for delete replication"
             );
-            return ReplicateDecision::default();
+            ReplicateDecision::default()
         }
+    }
+}
+
+pub(crate) async fn check_replicate_delete_strict(
+    bucket: &str,
+    dobj: &ObjectToDelete,
+    oi: &ObjectInfo,
+    del_opts: &ObjectOptions,
+    gerr: Option<String>,
+) -> Result<ReplicateDecision> {
+    let rcfg = match get_replication_config(bucket).await {
+        Ok(Some(config)) => config,
+        Ok(None) => return Ok(ReplicateDecision::default()),
+        Err(err) => return Err(err),
     };
 
     if del_opts.replication_request {
-        return ReplicateDecision::default();
+        return Ok(ReplicateDecision::default());
     }
 
-    if !del_opts.versioned {
-        return ReplicateDecision::default();
+    if !del_opts.versioned && !del_opts.version_suspended {
+        return Ok(ReplicateDecision::default());
     }
 
     let replication_delete = object_to_delete_for_replication(dobj);
@@ -209,7 +222,7 @@ pub(crate) async fn check_replicate_delete(
     let mut dsc = ReplicateDecision::new();
 
     if tgt_arns.is_empty() {
-        return dsc;
+        return Ok(dsc);
     }
 
     for tgt_arn in tgt_arns {
@@ -239,7 +252,7 @@ pub(crate) async fn check_replicate_delete(
         dsc.set(tgt_dsc);
     }
 
-    dsc
+    Ok(dsc)
 }
 
 pub(crate) async fn must_replicate(bucket: &str, object: &str, mopts: MustReplicateOptions) -> ReplicateDecision {

crates/ecstore/src/bucket/replication/replication_resyncer.rs

@@ -1151,60 +1151,6 @@ pub async fn replicate_delete<S: ReplicationStorage>(dobj: DeletedObjectReplicat
         dobj.delete_object.version_id
     };
 
-    let _rcfg = match get_replication_config(&bucket).await {
-        Ok(Some(config)) => config,
-        Ok(None) => {
-            debug!(
-                event = EVENT_REPLICATION_DELETE_SKIPPED,
-                component = LOG_COMPONENT_ECSTORE,
-                subsystem = LOG_SUBSYSTEM_REPLICATION_RESYNC,
-                bucket = %bucket,
-                reason = "replication_config_missing",
-                "Skipping replication delete because replication config is missing"
-            );
-            send_local_event(EventArgs {
-                event_name: EventName::ObjectReplicationNotTracked.to_string(),
-                bucket_name: bucket.clone(),
-                object: ObjectInfo {
-                    bucket: bucket.clone(),
-                    name: dobj.delete_object.object_name.clone(),
-                    version_id,
-                    delete_marker: dobj.delete_object.delete_marker,
-                    ..Default::default()
-                },
-                user_agent: "Internal: [Replication]".to_string(),
-                ..Default::default()
-            });
-
-            return;
-        }
-        Err(err) => {
-            debug!(
-                event = EVENT_REPLICATION_DELETE_SKIPPED,
-                component = LOG_COMPONENT_ECSTORE,
-                subsystem = LOG_SUBSYSTEM_REPLICATION_RESYNC,
-                bucket = %bucket,
-                error = %err,
-                reason = "replication_config_lookup_failed",
-                "Skipping replication delete because replication config lookup failed"
-            );
-            send_local_event(EventArgs {
-                event_name: EventName::ObjectReplicationNotTracked.to_string(),
-                bucket_name: bucket.clone(),
-                object: ObjectInfo {
-                    bucket: bucket.clone(),
-                    name: dobj.delete_object.object_name.clone(),
-                    version_id,
-                    delete_marker: dobj.delete_object.delete_marker,
-                    ..Default::default()
-                },
-                user_agent: "Internal: [Replication]".to_string(),
-                ..Default::default()
-            });
-            return;
-        }
-    };
-
     if dobj.delete_object.delete_marker
         && let Some(delete_marker_version_id) = dobj.delete_object.delete_marker_version_id
     {

crates/replication/src/config.rs

@@ -51,6 +51,28 @@ pub enum ReplicationTargetValidationError {
     StaleTarget,
 }
 
+pub fn unsupported_replication_config_field(config: &ReplicationConfiguration) -> Option<&'static str> {
+    for rule in &config.rules {
+        if rule
+            .source_selection_criteria
+            .as_ref()
+            .is_some_and(|criteria| criteria.sse_kms_encrypted_objects.is_some())
+        {
+            return Some("SourceSelectionCriteria.SseKmsEncryptedObjects");
+        }
+        if rule.destination.encryption_configuration.is_some() {
+            return Some("Destination.EncryptionConfiguration");
+        }
+        if rule.destination.metrics.is_some() {
+            return Some("Destination.Metrics");
+        }
+        if rule.destination.replication_time.is_some() {
+            return Some("Destination.ReplicationTime");
+        }
+    }
+    None
+}
+
 pub fn active_replication_rule_destination_arns(config: &ReplicationConfiguration) -> HashSet<String> {
     let mut arns = HashSet::new();
 
@@ -217,11 +239,6 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
                 }
 
                 if obj.version_id.is_some() {
-                    if obj.delete_marker {
-                        return rule.delete_marker_replication.clone().is_some_and(|d| {
-                            d.status == Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::ENABLED))
-                        });
-                    }
                     return rule
                         .delete_replication
                         .clone()
@@ -305,7 +322,11 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use s3s::dto::{DeleteMarkerReplication, Destination, ExistingObjectReplication, ReplicationRule};
+    use s3s::dto::{
+        DeleteMarkerReplication, DeleteReplication, Destination, EncryptionConfiguration, ExistingObjectReplication, Metrics,
+        MetricsStatus, ReplicationRule, ReplicationTime, ReplicationTimeStatus, ReplicationTimeValue, SourceSelectionCriteria,
+        SseKmsEncryptedObjects, SseKmsEncryptedObjectsStatus,
+    };
 
     fn replication_rule(id: &str, arn: &str) -> ReplicationRule {
         ReplicationRule {
@@ -605,4 +626,99 @@ mod tests {
             "highest-priority rule disables delete-marker replication, so the delete marker must not replicate"
         );
     }
+
+    #[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)),
+        });
+        rule.delete_replication = Some(DeleteReplication {
+            status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
+        });
+        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()
+            }));
+        }
+        assert!(!config.replicate(&ObjectOpts {
+            name: "object".to_string(),
+            delete_marker: true,
+            op_type: ReplicationType::Delete,
+            ..Default::default()
+        }));
+
+        let rule = &mut config.rules[0];
+        rule.delete_marker_replication = Some(DeleteMarkerReplication {
+            status: Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::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,
+            ..Default::default()
+        }));
+    }
+
+    #[test]
+    fn unsupported_replication_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("unsupported", arn)],
+        };
+
+        config.rules[0].source_selection_criteria = Some(SourceSelectionCriteria {
+            replica_modifications: None,
+            sse_kms_encrypted_objects: Some(SseKmsEncryptedObjects {
+                status: SseKmsEncryptedObjectsStatus::from_static(SseKmsEncryptedObjectsStatus::ENABLED),
+            }),
+        });
+        assert_eq!(
+            unsupported_replication_config_field(&config),
+            Some("SourceSelectionCriteria.SseKmsEncryptedObjects")
+        );
+
+        config.rules[0].source_selection_criteria = None;
+        config.rules[0].destination.encryption_configuration = Some(EncryptionConfiguration::default());
+        assert_eq!(unsupported_replication_config_field(&config), Some("Destination.EncryptionConfiguration"));
+
+        config.rules[0].destination.encryption_configuration = None;
+        config.rules[0].destination.metrics = Some(Metrics {
+            event_threshold: None,
+            status: MetricsStatus::from_static(MetricsStatus::ENABLED),
+        });
+        assert_eq!(unsupported_replication_config_field(&config), Some("Destination.Metrics"));
+
+        config.rules[0].destination.metrics = None;
+        config.rules[0].destination.replication_time = Some(ReplicationTime {
+            status: ReplicationTimeStatus::from_static(ReplicationTimeStatus::ENABLED),
+            time: ReplicationTimeValue { minutes: Some(15) },
+        });
+        assert_eq!(unsupported_replication_config_field(&config), Some("Destination.ReplicationTime"));
+    }
 }

crates/replication/src/lib.rs

@@ -30,7 +30,8 @@ pub mod tagging;
 
 pub use config::{
     ObjectOpts, ReplicationConfigurationExt, ReplicationTargetValidationError, active_replication_rule_destination_arns,
-    replication_target_arns, should_remove_replication_target, validate_replication_config_target_arns,
+    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

@@ -322,9 +322,9 @@ mod tests {
     use crate::storage_api::ObjectToDelete;
     use crate::{ReplicationStatusType, ReplicationType, VersionPurgeStatusType, target_reset_header};
     use s3s::dto::{
-        DeleteMarkerReplication, DeleteMarkerReplicationStatus, Destination, ExistingObjectReplication,
-        ExistingObjectReplicationStatus, ReplicaModifications, ReplicaModificationsStatus, ReplicationConfiguration,
-        ReplicationRule, ReplicationRuleStatus, SourceSelectionCriteria,
+        DeleteMarkerReplication, DeleteMarkerReplicationStatus, DeleteReplication, DeleteReplicationStatus, Destination,
+        ExistingObjectReplication, ExistingObjectReplicationStatus, ReplicaModifications, ReplicaModificationsStatus,
+        ReplicationConfiguration, ReplicationRule, ReplicationRuleStatus, SourceSelectionCriteria,
     };
     use std::collections::HashMap;
     use time::{Duration, OffsetDateTime};
@@ -433,7 +433,9 @@ mod tests {
             delete_marker_replication: Some(DeleteMarkerReplication {
                 status: Some(DeleteMarkerReplicationStatus::from_static(DeleteMarkerReplicationStatus::ENABLED)),
             }),
-            delete_replication: None,
+            delete_replication: Some(DeleteReplication {
+                status: DeleteReplicationStatus::from_static(DeleteReplicationStatus::ENABLED),
+            }),
             destination: Destination {
                 bucket: arn.to_string(),
                 ..Default::default()
@@ -513,7 +515,7 @@ mod tests {
         };
 
         let state = delete_replication_state_from_config(&config, &source)
-            .expect("delete-marker version purge should honor delete-marker replication rules");
+            .expect("delete-marker version purge should honor delete replication rules");
         let pending = format!("{arn}=PENDING;");
 
         assert_eq!(state.version_purge_status_internal.as_deref(), Some(pending.as_str()));