mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-31 09:18:28 +00:00
fix(replication): harden bucket replication correctness (#4116)
This commit is contained in:
@@ -49,9 +49,10 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
|
||||
/// Check whether any object-replication rules exist
|
||||
fn has_existing_object_replication(&self, arn: &str) -> (bool, bool) {
|
||||
let mut has_arn = false;
|
||||
let arn = arn.trim();
|
||||
|
||||
for rule in &self.rules {
|
||||
if rule.destination.bucket == arn || self.role == arn {
|
||||
if rule.destination.bucket.trim() == arn || self.role.trim() == arn {
|
||||
if !has_arn {
|
||||
has_arn = true;
|
||||
}
|
||||
@@ -77,7 +78,10 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
|
||||
continue;
|
||||
}
|
||||
|
||||
if !obj.target_arn.is_empty() && rule.destination.bucket != obj.target_arn && self.role != obj.target_arn {
|
||||
if !obj.target_arn.is_empty()
|
||||
&& rule.destination.bucket.trim() != obj.target_arn.trim()
|
||||
&& self.role.trim() != obj.target_arn.trim()
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -215,6 +219,11 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
|
||||
|
||||
/// Filter target ARNs and return a slice of the distinct values in the config
|
||||
fn filter_target_arns(&self, obj: &ObjectOpts) -> Vec<String> {
|
||||
let role = self.role.trim();
|
||||
if !role.is_empty() {
|
||||
return vec![role.to_string()];
|
||||
}
|
||||
|
||||
let mut arns = Vec::new();
|
||||
let mut targets_map: HashSet<String> = HashSet::new();
|
||||
let rules = self.filter_actionable_rules(obj);
|
||||
@@ -224,16 +233,12 @@ impl ReplicationConfigurationExt for ReplicationConfiguration {
|
||||
continue;
|
||||
}
|
||||
|
||||
if !rule.destination.bucket.is_empty() && !targets_map.contains(&rule.destination.bucket) {
|
||||
targets_map.insert(rule.destination.bucket.clone());
|
||||
let arn = rule.destination.bucket.trim();
|
||||
if !arn.is_empty() && !targets_map.contains(arn) {
|
||||
targets_map.insert(arn.to_string());
|
||||
}
|
||||
}
|
||||
|
||||
if targets_map.is_empty() && !self.role.is_empty() {
|
||||
arns.push(self.role.clone());
|
||||
return arns;
|
||||
}
|
||||
|
||||
for arn in targets_map {
|
||||
arns.push(arn);
|
||||
}
|
||||
@@ -267,9 +272,9 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_target_arns_keeps_multiple_destinations_when_role_is_present() {
|
||||
fn filter_target_arns_uses_role_when_role_is_present() {
|
||||
let config = ReplicationConfiguration {
|
||||
role: "arn:legacy:target".to_string(),
|
||||
role: " arn:legacy:target ".to_string(),
|
||||
rules: vec![
|
||||
replication_rule("rule-1", "arn:target:a"),
|
||||
replication_rule("rule-2", "arn:target:b"),
|
||||
@@ -282,9 +287,7 @@ mod tests {
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
assert_eq!(arns.len(), 2);
|
||||
assert!(arns.iter().any(|arn| arn == "arn:target:a"));
|
||||
assert!(arns.iter().any(|arn| arn == "arn:target:b"));
|
||||
assert_eq!(arns, vec!["arn:legacy:target".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1424,7 +1424,7 @@ pub fn resync_target(
|
||||
|
||||
if rs.is_none() {
|
||||
let reset_before_date = reset_before_date.unwrap_or(OffsetDateTime::UNIX_EPOCH);
|
||||
if !reset_id.is_empty() && mod_time < reset_before_date {
|
||||
if !reset_id.is_empty() && mod_time <= reset_before_date {
|
||||
dec.replicate = true;
|
||||
return dec;
|
||||
}
|
||||
@@ -1447,13 +1447,13 @@ pub fn resync_target(
|
||||
return dec;
|
||||
}
|
||||
|
||||
let new_reset = parts[0] == reset_id;
|
||||
let new_reset = parts[1] != reset_id;
|
||||
|
||||
if !new_reset && status == ReplicationStatusType::Completed {
|
||||
return dec;
|
||||
}
|
||||
|
||||
dec.replicate = new_reset && mod_time < reset_before_date;
|
||||
dec.replicate = new_reset && mod_time <= reset_before_date;
|
||||
|
||||
dec
|
||||
}
|
||||
@@ -3337,10 +3337,7 @@ impl ReplicateObjectInfoExt for ReplicateObjectInfo {
|
||||
rinfo.replication_status = ReplicationStatusType::Completed;
|
||||
if replication_action == ReplicationAction::None {
|
||||
if self.op_type == ReplicationType::ExistingObject
|
||||
&& object_info.mod_time
|
||||
> oi.last_modified
|
||||
.map(|dt| OffsetDateTime::from_unix_timestamp(dt.secs()).unwrap_or(OffsetDateTime::UNIX_EPOCH))
|
||||
&& object_info.version_id.is_none()
|
||||
&& target_is_newer_than_source_null_version(&object_info, &oi)
|
||||
{
|
||||
warn!(
|
||||
event = EVENT_RESYNC_RUNTIME_SKIPPED,
|
||||
@@ -3350,7 +3347,7 @@ impl ReplicateObjectInfoExt for ReplicateObjectInfo {
|
||||
object = %object,
|
||||
arn = %tgt_client.arn,
|
||||
endpoint = %tgt_client.to_url(),
|
||||
reason = "newer_target_version_exists",
|
||||
reason = "target_newer_than_source_null_version",
|
||||
"Skipping replication because newer target version exists"
|
||||
);
|
||||
send_local_event(EventArgs {
|
||||
@@ -3558,7 +3555,7 @@ impl ReplicateObjectInfoExt for ReplicateObjectInfo {
|
||||
replication_status: self.replication_status.clone(),
|
||||
version_purge_status_internal: self.version_purge_status_internal.clone(),
|
||||
version_purge_status: self.version_purge_status.clone(),
|
||||
delete_marker: true,
|
||||
delete_marker: self.delete_marker,
|
||||
checksum: self.checksum.clone(),
|
||||
..Default::default()
|
||||
}
|
||||
@@ -3983,14 +3980,15 @@ async fn replicate_object_with_multipart<S: ReplicationObjectIO>(ctx: MultipartR
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn get_replication_action(oi1: &ObjectInfo, oi2: &HeadObjectOutput, op_type: ReplicationType) -> ReplicationAction {
|
||||
if op_type == ReplicationType::ExistingObject
|
||||
&& oi1.mod_time
|
||||
> oi2
|
||||
.last_modified
|
||||
.map(|dt| OffsetDateTime::from_unix_timestamp(dt.secs()).unwrap_or(OffsetDateTime::UNIX_EPOCH))
|
||||
fn target_is_newer_than_source_null_version(oi1: &ObjectInfo, oi2: &HeadObjectOutput) -> bool {
|
||||
oi2.last_modified
|
||||
.map(|dt| OffsetDateTime::from_unix_timestamp(dt.secs()).unwrap_or(OffsetDateTime::UNIX_EPOCH))
|
||||
.is_some_and(|target_mod_time| target_mod_time > oi1.mod_time.unwrap_or(OffsetDateTime::UNIX_EPOCH))
|
||||
&& oi1.version_id.is_none()
|
||||
{
|
||||
}
|
||||
|
||||
fn get_replication_action(oi1: &ObjectInfo, oi2: &HeadObjectOutput, op_type: ReplicationType) -> ReplicationAction {
|
||||
if op_type == ReplicationType::ExistingObject && target_is_newer_than_source_null_version(oi1, oi2) {
|
||||
return ReplicationAction::None;
|
||||
}
|
||||
|
||||
@@ -4093,8 +4091,9 @@ fn get_replication_action(oi1: &ObjectInfo, oi2: &HeadObjectOutput, op_type: Rep
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use aws_smithy_types::DateTime;
|
||||
use std::collections::HashMap;
|
||||
use time::OffsetDateTime;
|
||||
use time::{Duration, OffsetDateTime};
|
||||
use uuid::Uuid;
|
||||
|
||||
#[test]
|
||||
@@ -4356,6 +4355,107 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_replication_action_existing_object_source_newer_null_version_requires_replication() {
|
||||
let source = ObjectInfo {
|
||||
mod_time: Some(OffsetDateTime::UNIX_EPOCH + Duration::seconds(20)),
|
||||
version_id: None,
|
||||
..Default::default()
|
||||
};
|
||||
let target = HeadObjectOutput::builder().last_modified(DateTime::from_secs(10)).build();
|
||||
|
||||
assert_eq!(
|
||||
get_replication_action(&source, &target, ReplicationType::ExistingObject),
|
||||
ReplicationAction::All,
|
||||
"a newer source null version must not be skipped during existing-object replication"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_get_replication_action_existing_object_target_newer_null_version_skips() {
|
||||
let source = ObjectInfo {
|
||||
mod_time: Some(OffsetDateTime::UNIX_EPOCH + Duration::seconds(10)),
|
||||
version_id: None,
|
||||
..Default::default()
|
||||
};
|
||||
let target = HeadObjectOutput::builder().last_modified(DateTime::from_secs(20)).build();
|
||||
|
||||
assert_eq!(
|
||||
get_replication_action(&source, &target, ReplicationType::ExistingObject),
|
||||
ReplicationAction::None,
|
||||
"a newer target null-version object should not be overwritten by existing-object replication"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resync_target_includes_object_at_reset_before_boundary() {
|
||||
let reset_before = OffsetDateTime::UNIX_EPOCH + Duration::seconds(30);
|
||||
let oi = ObjectInfo {
|
||||
mod_time: Some(reset_before),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let decision = resync_target(&oi, "arn:target", "reset-1", Some(reset_before), ReplicationStatusType::Completed);
|
||||
|
||||
assert!(
|
||||
decision.replicate,
|
||||
"objects whose mod_time equals reset_before must be included in the reset window"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resync_target_replicates_when_reset_id_changes() {
|
||||
let reset_before = OffsetDateTime::UNIX_EPOCH + Duration::seconds(30);
|
||||
let oi = ObjectInfo {
|
||||
mod_time: Some(OffsetDateTime::UNIX_EPOCH + Duration::seconds(10)),
|
||||
user_defined: Arc::new(HashMap::from([(
|
||||
target_reset_header("arn:target"),
|
||||
"1970-01-01T00:00:20Z;old-reset".to_string(),
|
||||
)])),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let decision = resync_target(&oi, "arn:target", "new-reset", Some(reset_before), ReplicationStatusType::Completed);
|
||||
|
||||
assert!(decision.replicate, "a new reset id must resync objects marked by an older reset");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resync_target_skips_completed_object_for_same_reset_id() {
|
||||
let reset_before = OffsetDateTime::UNIX_EPOCH + Duration::seconds(30);
|
||||
let oi = ObjectInfo {
|
||||
mod_time: Some(OffsetDateTime::UNIX_EPOCH + Duration::seconds(10)),
|
||||
user_defined: Arc::new(HashMap::from([(
|
||||
target_reset_header("arn:target"),
|
||||
"1970-01-01T00:00:20Z;same-reset".to_string(),
|
||||
)])),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let decision = resync_target(&oi, "arn:target", "same-reset", Some(reset_before), ReplicationStatusType::Completed);
|
||||
|
||||
assert!(!decision.replicate, "the same completed reset id must not resync again");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_replicate_object_info_to_object_info_preserves_delete_marker_flag() {
|
||||
let live = ReplicateObjectInfo {
|
||||
bucket: "source".to_string(),
|
||||
name: "object".to_string(),
|
||||
delete_marker: false,
|
||||
..Default::default()
|
||||
};
|
||||
let delete_marker = ReplicateObjectInfo {
|
||||
bucket: "source".to_string(),
|
||||
name: "object".to_string(),
|
||||
delete_marker: true,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(!live.to_object_info().delete_marker);
|
||||
assert!(delete_marker.to_object_info().delete_marker);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_delete_replication_object_opts_marks_replica_deletes() {
|
||||
let dobj = ObjectToDelete {
|
||||
|
||||
Reference in New Issue
Block a user