fix(storage): harden rebalance decommission state (#3515)

crates/ecstore/src/store/object.rs

@@ -216,6 +216,10 @@ fn resolve_latest_object_access(
     Ok((info, idx))
 }
 
+fn should_create_delete_marker_for_missing_object(opts: &ObjectOptions) -> bool {
+    opts.versioned && opts.version_id.is_none() && !opts.delete_marker && !opts.data_movement
+}
+
 fn version_aware_lookup_opts(opts: &ObjectOptions, no_lock: bool) -> ObjectOptions {
     let mut lookup_opts = opts.clone();
     lookup_opts.no_lock = no_lock;
@@ -234,6 +238,12 @@ fn data_movement_pool_lookup_opts(opts: &ObjectOptions, no_lock: bool) -> Object
     lookup_opts
 }
 
+fn transition_restore_pool_opts(opts: &ObjectOptions) -> ObjectOptions {
+    let mut lookup_opts = opts.clone();
+    lookup_opts.skip_decommissioned = true;
+    lookup_opts
+}
+
 fn effective_object_actual_size(info: &ObjectInfo) -> Option<i64> {
     info.get_actual_size().ok()
 }
@@ -243,27 +253,44 @@ fn is_equivalent_data_movement_delete_marker(source: &ObjectInfo, target: &Objec
         && is_data_movement_delete_marker(target)
         && source.version_id == target.version_id
         && source.mod_time == target.mod_time
+        && source.user_defined == target.user_defined
+        && source.user_tags == target.user_tags
+        && source.replication_status_internal == target.replication_status_internal
+        && source.replication_status == target.replication_status
+        && source.version_purge_status_internal == target.version_purge_status_internal
+        && source.version_purge_status == target.version_purge_status
 }
 
 fn is_data_movement_delete_marker(info: &ObjectInfo) -> bool {
     info.delete_marker
 }
 
+fn expected_data_movement_tiered_object(source: &rustfs_filemeta::FileInfo) -> ObjectInfo {
+    ObjectInfo::from_file_info(source, "", &source.name, source.version_id.is_some())
+}
+
 fn is_equivalent_data_movement_tiered_object(source: &rustfs_filemeta::FileInfo, target: &ObjectInfo) -> bool {
+    let expected = expected_data_movement_tiered_object(source);
+
     source.version_id == target.version_id
         && !target.delete_marker
         && source.size == target.size
         && source.get_etag() == target.etag
         && source.checksum == target.checksum
         && source.mod_time == target.mod_time
-        && source.transition_status == target.transitioned_object.status
-        && source.transitioned_objname == target.transitioned_object.name
-        && source.transition_tier == target.transitioned_object.tier
-        && source
-            .transition_version_id
-            .map(|version_id| version_id.to_string())
-            .unwrap_or_default()
-            == target.transitioned_object.version_id
+        && expected.user_defined == target.user_defined
+        && expected.user_tags == target.user_tags
+        && expected.expires == target.expires
+        && expected.storage_class == target.storage_class
+        && expected.replication_status_internal == target.replication_status_internal
+        && expected.replication_status == target.replication_status
+        && expected.version_purge_status_internal == target.version_purge_status_internal
+        && expected.version_purge_status == target.version_purge_status
+        && expected.transitioned_object.status == target.transitioned_object.status
+        && expected.transitioned_object.name == target.transitioned_object.name
+        && expected.transitioned_object.tier == target.transitioned_object.tier
+        && expected.transitioned_object.version_id == target.transitioned_object.version_id
+        && expected.transitioned_object.free_version == target.transitioned_object.free_version
         && effective_object_actual_size(target) == Some(source.size)
 }
 
@@ -836,7 +863,7 @@ impl ECStore {
             let target_pool_idx =
                 resolve_data_movement_resume_target_pool(selected_target_pool_idx, resume_target_pool_idx, opts.src_pool_idx);
 
-            if opts.src_pool_idx == selected_target_pool_idx {
+            if !should_check_data_movement_resume_target(opts.src_pool_idx, target_pool_idx) {
                 if let Ok((source_pool_info, _)) = existing_pool_info
                     && opts.delete_marker
                     && is_data_movement_delete_marker(&source_pool_info.object_info)
@@ -862,24 +889,23 @@ impl ECStore {
                 ));
             }
 
-            let mut obj = self.pools[selected_target_pool_idx]
-                .delete_object(bucket, object, opts)
-                .await?;
+            let mut obj = self.pools[target_pool_idx].delete_object(bucket, object, opts).await?;
             obj.name = decode_dir_object(obj.name.as_str());
             return Ok(obj);
         }
 
         // Determine which pool contains it
-        let (mut pinfo, errs) = self
-            .get_pool_info_existing_with_opts(bucket, object, &gopts)
-            .await
-            .map_err(|e| {
-                if is_err_read_quorum(&e) {
-                    StorageError::ErasureWriteQuorum
-                } else {
-                    e
-                }
-            })?;
+        let (mut pinfo, errs) = match self.get_pool_info_existing_with_opts(bucket, object, &gopts).await {
+            Ok(res) => res,
+            Err(err) if is_err_read_quorum(&err) => return Err(StorageError::ErasureWriteQuorum),
+            Err(err) if is_err_object_not_found(&err) && should_create_delete_marker_for_missing_object(&opts) => {
+                let target_pool_idx = self.get_pool_idx_no_lock(bucket, object, 0).await?;
+                let mut obj = self.pools[target_pool_idx].delete_object(bucket, object, opts).await?;
+                obj.name = decode_dir_object(object);
+                return Ok(obj);
+            }
+            Err(err) => return Err(err),
+        };
 
         if pinfo.object_info.delete_marker && opts.version_id.is_none() {
             pinfo.object_info.name = decode_dir_object(object);
@@ -1133,11 +1159,12 @@ impl ECStore {
             return self.pools[0].transition_object(bucket, &object, opts).await;
         }
 
-        //opts.skip_decommissioned = true;
-        //opts.no_lock = true;
-        let (_, idx) = self.get_latest_accessible_object_info_with_idx(bucket, &object, opts).await?;
+        let opts = transition_restore_pool_opts(opts);
+        let (_, idx) = self
+            .get_latest_accessible_object_info_with_idx(bucket, &object, &opts)
+            .await?;
 
-        self.pools[idx].transition_object(bucket, &object, opts).await
+        self.pools[idx].transition_object(bucket, &object, &opts).await
     }
 
     #[instrument(skip(self))]
@@ -1152,15 +1179,14 @@ impl ECStore {
             return self.pools[0].clone().restore_transitioned_object(bucket, &object, opts).await;
         }
 
-        //opts.skip_decommissioned = true;
-        //opts.nolock = true;
+        let opts = transition_restore_pool_opts(opts);
         let (_, idx) = self
-            .get_latest_accessible_object_info_with_idx(bucket, object.as_str(), opts)
+            .get_latest_accessible_object_info_with_idx(bucket, object.as_str(), &opts)
             .await?;
 
         self.pools[idx]
             .clone()
-            .restore_transitioned_object(bucket, &object, opts)
+            .restore_transitioned_object(bucket, &object, &opts)
             .await
     }
 
@@ -1269,8 +1295,8 @@ mod tests {
     use super::*;
     use crate::bucket::lifecycle::core::TRANSITION_COMPLETE;
     use bytes::Bytes;
-    use rustfs_storage_api::TransitionedObject;
     use std::io::Cursor;
+    use std::sync::Arc;
     use tokio::io::AsyncReadExt;
 
     #[test]
@@ -1313,6 +1339,33 @@ mod tests {
         assert!(!is_equivalent_data_movement_delete_marker(&source, &mismatched));
     }
 
+    #[test]
+    fn equivalent_data_movement_delete_marker_rejects_metadata_and_replication_mismatch() {
+        let version_id = Uuid::nil();
+        let mod_time = OffsetDateTime::UNIX_EPOCH;
+        let source = ObjectInfo {
+            version_id: Some(version_id),
+            delete_marker: true,
+            mod_time: Some(mod_time),
+            user_defined: Arc::new(HashMap::from([("x-amz-meta-source".to_string(), "true".to_string())])),
+            replication_status_internal: Some("arn:minio:replication:target=COMPLETED;".to_string()),
+            version_purge_status_internal: Some("arn:minio:replication:target=PENDING;".to_string()),
+            ..Default::default()
+        };
+
+        let mut target = source.clone();
+        target.user_defined = Arc::new(HashMap::from([("x-amz-meta-source".to_string(), "false".to_string())]));
+        assert!(!is_equivalent_data_movement_delete_marker(&source, &target));
+
+        let mut target = source.clone();
+        target.replication_status_internal = Some("arn:minio:replication:target=FAILED;".to_string());
+        assert!(!is_equivalent_data_movement_delete_marker(&source, &target));
+
+        let mut target = source.clone();
+        target.version_purge_status_internal = Some("arn:minio:replication:target=COMPLETE;".to_string());
+        assert!(!is_equivalent_data_movement_delete_marker(&source, &target));
+    }
+
     #[test]
     fn equivalent_data_movement_delete_marker_rejects_live_object() {
         let source = ObjectInfo {
@@ -1367,6 +1420,7 @@ mod tests {
     fn data_movement_resume_target_uses_resolved_non_source_pool_when_selected_is_source() {
         let target_pool_idx = resolve_data_movement_resume_target_pool(1, Some(3), 1);
         assert_eq!(target_pool_idx, 3);
+        assert!(should_check_data_movement_resume_target(1, target_pool_idx));
     }
 
     #[test]
@@ -1386,12 +1440,12 @@ mod tests {
         assert!(matches!(result, Err(Error::SlowDown)));
     }
 
-    #[test]
-    fn equivalent_data_movement_tiered_object_accepts_matching_transition_metadata() {
+    fn tiered_equivalence_source() -> FileInfo {
         let version_id = Uuid::nil();
         let transition_version_id = Uuid::new_v4();
         let mod_time = OffsetDateTime::UNIX_EPOCH;
-        let source = FileInfo {
+
+        FileInfo {
             version_id: Some(version_id),
             size: 1024,
             mod_time: Some(mod_time),
@@ -1400,80 +1454,86 @@ mod tests {
             transitioned_objname: "remote/object".to_string(),
             transition_tier: "WARM".to_string(),
             transition_version_id: Some(transition_version_id),
-            metadata: HashMap::from([("etag".to_string(), "etag-value".to_string())]),
-            ..Default::default()
-        };
-        let target = ObjectInfo {
-            version_id: Some(version_id),
-            size: 1024,
-            mod_time: Some(mod_time),
-            checksum: Some(Bytes::from_static(b"checksum")),
-            etag: Some("etag-value".to_string()),
-            transitioned_object: TransitionedObject {
-                name: "remote/object".to_string(),
-                version_id: transition_version_id.to_string(),
-                tier: "WARM".to_string(),
-                status: TRANSITION_COMPLETE.to_string(),
+            replication_state_internal: Some(rustfs_filemeta::ReplicationState {
+                replication_status_internal: Some("arn:minio:replication:target=COMPLETED;".to_string()),
+                targets: rustfs_filemeta::replication_statuses_map("arn:minio:replication:target=COMPLETED;"),
+                version_purge_status_internal: Some("arn:minio:replication:target=PENDING;".to_string()),
+                purge_targets: rustfs_filemeta::version_purge_statuses_map("arn:minio:replication:target=PENDING;"),
                 ..Default::default()
-            },
+            }),
+            metadata: HashMap::from([
+                ("etag".to_string(), "etag-value".to_string()),
+                ("x-amz-meta-key".to_string(), "metadata-value".to_string()),
+                (rustfs_utils::http::AMZ_OBJECT_TAGGING.to_string(), "tag=value".to_string()),
+                ("expires".to_string(), "1970-01-01T00:33:20Z".to_string()),
+            ]),
             ..Default::default()
-        };
+        }
+    }
+
+    fn tiered_equivalence_target(source: &FileInfo) -> ObjectInfo {
+        ObjectInfo::from_file_info(source, "bucket", "object", source.version_id.is_some())
+    }
+
+    #[test]
+    fn equivalent_data_movement_tiered_object_accepts_matching_persisted_metadata() {
+        let source = tiered_equivalence_source();
+        let target = tiered_equivalence_target(&source);
 
         assert!(is_equivalent_data_movement_tiered_object(&source, &target));
     }
 
     #[test]
     fn equivalent_data_movement_tiered_object_rejects_transition_mismatch() {
-        let source = FileInfo {
-            version_id: Some(Uuid::nil()),
-            size: 1024,
-            transition_status: TRANSITION_COMPLETE.to_string(),
-            transitioned_objname: "remote/source".to_string(),
-            transition_tier: "WARM".to_string(),
-            ..Default::default()
-        };
-        let target = ObjectInfo {
-            version_id: source.version_id,
-            size: 1024,
-            transitioned_object: TransitionedObject {
-                name: "remote/target".to_string(),
-                tier: "WARM".to_string(),
-                status: TRANSITION_COMPLETE.to_string(),
-                ..Default::default()
-            },
-            ..Default::default()
-        };
+        let source = tiered_equivalence_source();
+        let mut target = tiered_equivalence_target(&source);
+        target.transitioned_object.name = "remote/target".to_string();
+
+        assert!(!is_equivalent_data_movement_tiered_object(&source, &target));
+    }
+
+    #[test]
+    fn equivalent_data_movement_tiered_object_rejects_user_metadata_mismatch() {
+        let source = tiered_equivalence_source();
+        let mut target = tiered_equivalence_target(&source);
+        target.user_defined = Arc::new(HashMap::from([("x-amz-meta-key".to_string(), "target-value".to_string())]));
+
+        assert!(!is_equivalent_data_movement_tiered_object(&source, &target));
+    }
+
+    #[test]
+    fn equivalent_data_movement_tiered_object_rejects_tag_mismatch() {
+        let source = tiered_equivalence_source();
+        let mut target = tiered_equivalence_target(&source);
+        target.user_tags = Arc::new("tag=target".to_string());
+
+        assert!(!is_equivalent_data_movement_tiered_object(&source, &target));
+    }
+
+    #[test]
+    fn equivalent_data_movement_tiered_object_rejects_replication_mismatch() {
+        let source = tiered_equivalence_source();
+        let mut target = tiered_equivalence_target(&source);
+        target.replication_status_internal = Some("arn:minio:replication:target=FAILED;".to_string());
+        target.replication_status = rustfs_filemeta::ReplicationStatusType::Failed;
+
+        assert!(!is_equivalent_data_movement_tiered_object(&source, &target));
+    }
+
+    #[test]
+    fn equivalent_data_movement_tiered_object_rejects_version_purge_mismatch() {
+        let source = tiered_equivalence_source();
+        let mut target = tiered_equivalence_target(&source);
+        target.version_purge_status_internal = Some("arn:minio:replication:target=COMPLETE;".to_string());
+        target.version_purge_status = rustfs_filemeta::VersionPurgeStatusType::Complete;
 
         assert!(!is_equivalent_data_movement_tiered_object(&source, &target));
     }
 
     #[test]
     fn data_movement_tiered_resume_accepts_equivalent_target() {
-        let version_id = Uuid::nil();
-        let transition_version_id = Uuid::new_v4();
-        let source = FileInfo {
-            version_id: Some(version_id),
-            size: 1024,
-            transition_status: TRANSITION_COMPLETE.to_string(),
-            transitioned_objname: "remote/object".to_string(),
-            transition_tier: "WARM".to_string(),
-            transition_version_id: Some(transition_version_id),
-            metadata: HashMap::from([("etag".to_string(), "etag-value".to_string())]),
-            ..Default::default()
-        };
-        let target = ObjectInfo {
-            version_id: Some(version_id),
-            size: 1024,
-            etag: Some("etag-value".to_string()),
-            transitioned_object: TransitionedObject {
-                name: "remote/object".to_string(),
-                version_id: transition_version_id.to_string(),
-                tier: "WARM".to_string(),
-                status: TRANSITION_COMPLETE.to_string(),
-                ..Default::default()
-            },
-            ..Default::default()
-        };
+        let source = tiered_equivalence_source();
+        let target = tiered_equivalence_target(&source);
 
         let should_resume = resolve_data_movement_tiered_resume_result(Ok(Some(target)), &source, 0, 1)
             .expect("equivalent tiered target should be evaluated");
@@ -1483,28 +1543,8 @@ mod tests {
 
     #[test]
     fn data_movement_tiered_resume_rejects_source_pool_target() {
-        let version_id = Uuid::nil();
-        let source = FileInfo {
-            version_id: Some(version_id),
-            size: 1024,
-            transition_status: TRANSITION_COMPLETE.to_string(),
-            transitioned_objname: "remote/object".to_string(),
-            transition_tier: "WARM".to_string(),
-            metadata: HashMap::from([("etag".to_string(), "etag-value".to_string())]),
-            ..Default::default()
-        };
-        let target = ObjectInfo {
-            version_id: Some(version_id),
-            size: 1024,
-            etag: Some("etag-value".to_string()),
-            transitioned_object: TransitionedObject {
-                name: "remote/object".to_string(),
-                tier: "WARM".to_string(),
-                status: TRANSITION_COMPLETE.to_string(),
-                ..Default::default()
-            },
-            ..Default::default()
-        };
+        let source = tiered_equivalence_source();
+        let target = tiered_equivalence_target(&source);
 
         let should_resume = resolve_data_movement_tiered_resume_result(Ok(Some(target)), &source, 0, 0)
             .expect("source-pool target should be rejected before target lookup");
@@ -1623,6 +1663,39 @@ mod tests {
         assert!(matches!(err, Error::MethodNotAllowed));
     }
 
+    #[test]
+    fn should_create_delete_marker_for_missing_object_allows_latest_versioned_delete() {
+        let opts = ObjectOptions {
+            versioned: true,
+            ..Default::default()
+        };
+
+        assert!(should_create_delete_marker_for_missing_object(&opts));
+    }
+
+    #[test]
+    fn should_create_delete_marker_for_missing_object_rejects_specialized_deletes() {
+        let version_delete = ObjectOptions {
+            versioned: true,
+            version_id: Some("vid-1".to_string()),
+            ..Default::default()
+        };
+        let delete_marker_replication = ObjectOptions {
+            versioned: true,
+            delete_marker: true,
+            ..Default::default()
+        };
+        let data_movement = ObjectOptions {
+            versioned: true,
+            data_movement: true,
+            ..Default::default()
+        };
+
+        assert!(!should_create_delete_marker_for_missing_object(&version_delete));
+        assert!(!should_create_delete_marker_for_missing_object(&delete_marker_replication));
+        assert!(!should_create_delete_marker_for_missing_object(&data_movement));
+    }
+
     #[test]
     fn resolve_decommission_target_pool_idx_result_passthrough_ok() {
         let idx = ECStore::resolve_decommission_target_pool_idx_result(Ok(3), "bucket", "object").unwrap();
@@ -1715,6 +1788,29 @@ mod tests {
         assert!(lookup_opts.skip_rebalancing);
     }
 
+    #[test]
+    fn transition_restore_pool_opts_skips_decommissioned_and_preserves_locking() {
+        let lookup_opts = transition_restore_pool_opts(&ObjectOptions {
+            no_lock: false,
+            skip_decommissioned: false,
+            ..Default::default()
+        });
+
+        assert!(lookup_opts.skip_decommissioned);
+        assert!(!lookup_opts.no_lock);
+    }
+
+    #[test]
+    fn transition_restore_pool_opts_preserves_existing_no_lock() {
+        let lookup_opts = transition_restore_pool_opts(&ObjectOptions {
+            no_lock: true,
+            ..Default::default()
+        });
+
+        assert!(lookup_opts.skip_decommissioned);
+        assert!(lookup_opts.no_lock);
+    }
+
     #[tokio::test]
     #[serial_test::serial]
     async fn reader_lock_is_held_when_optimization_is_disabled() {