diff --git a/crates/e2e_test/src/replication_extension_test.rs b/crates/e2e_test/src/replication_extension_test.rs index 1dc84c93d..92f6e166a 100644 --- a/crates/e2e_test/src/replication_extension_test.rs +++ b/crates/e2e_test/src/replication_extension_test.rs @@ -9289,11 +9289,13 @@ async fn test_replication_check_flags_version_minting_target() -> TestResult { fidelity["Code"], "BucketRemoteTargetVersionMismatch", "the failure must carry a machine-readable code: {payload}" ); - // The probe PUT itself succeeded (fidelity is judged from its response); - // the later mutation phases are pointless against a drifting target and - // must be skipped, but cleanup still runs. + // The probe PUT itself succeeded (fidelity is judged from its response). + // The mutation phases address the id the target assigned — the ledger + // the worker records per object (rustfs/backlog#2340) — so they run and + // pass on a drifting target, and cleanup uses the same id. assert_eq!(target_report["Phases"]["Put"]["Status"], "OK", "{payload}"); - assert_eq!(target_report["Phases"]["DeleteMarker"]["Status"], "SKIPPED", "{payload}"); + assert_eq!(target_report["Phases"]["DeleteMarker"]["Status"], "OK", "{payload}"); + assert_eq!(target_report["Phases"]["VersionDelete"]["Status"], "OK", "{payload}"); assert_eq!(target_report["Phases"]["Cleanup"]["Status"], "OK", "{payload}"); // The probe PUT must carry the source version as `?versionId=` — the diff --git a/docs/operations/replication-check.md b/docs/operations/replication-check.md index d102d30db..fcada857f 100644 --- a/docs/operations/replication-check.md +++ b/docs/operations/replication-check.md @@ -62,4 +62,4 @@ The route returns HTTP 200 with JSON after all configured targets have been chec `VersionFidelity` pins the version-identity contract on both write paths. The probe PUT carries a source version id (header plus `?versionId=` query, the exact shape live replication uses) and the target must answer with the same id; a second probe repeats the check through CreateMultipartUpload -> UploadPart -> CompleteMultipartUpload, where the target fixes the version at initiate and only reports it on completion. A target can adopt PutObject ids and still mint its own for multipart; the failure message names the path that drifted. -A target that mints its own version ids breaks every version-addressed operation that follows (version deletes, heal re-drives). The phase therefore fails with `"Code": "BucketRemoteTargetVersionMismatch"`, the later mutation phases are skipped, and cleanup still removes the probe via the version id the target actually assigned. +A target that mints its own version ids never answers to the source version id. The phase therefore fails with `"Code": "BucketRemoteTargetVersionMismatch"` and the target result is `FAILED`. Replication to such a target still converges: the replication worker records the id the target assigned to each object version on the source (the target-version ledger, internal metadata key `replication-target-version-`) and addresses version deletes, tag and Object Lock updates through it. The `DeleteMarker` and `VersionDelete` phases probe exactly that path — they address the id the target assigned to the probe object, not the source id — so on a drifting target they report whether ledger-addressed purges work against this endpoint, and cleanup removes the probe via the same id. They are `SKIPPED` only when the probe `Put` itself failed or reported no version id. diff --git a/rustfs/src/admin/router.rs b/rustfs/src/admin/router.rs index bb0e06f72..8b990ff56 100644 --- a/rustfs/src/admin/router.rs +++ b/rustfs/src/admin/router.rs @@ -2245,12 +2245,13 @@ async fn execute_replication_probe(result: &mut ReplicationCheckTargetStatus, op match operations.put().await { Ok(outcome) => { result.phases.put = ReplicationCheckPhaseStatus::passed(); - // P1-19 version-identity contract: replication only converges on - // targets that adopt the source version id — version-addressed - // deletes and heal re-drives never match a minted id. Judge it - // from the probe PUT's own response; on mismatch the later - // mutation phases are pointless (they address by version id), but - // cleanup still runs against whatever id the target assigned. + // P1-19 version-identity contract: a target that mints its own + // version ids never answers to the source id. Judge it from the + // probe PUT's own response. The mutation phases below still run + // on such a target: they address the id the target assigned — + // the same ledger the replication worker records per object + // (rustfs/backlog#2340) — so they report whether version- + // addressed deletes can converge there at all. match version_fidelity_error("PutObject", &outcome) { None => result.phases.version_fidelity = ReplicationCheckPhaseStatus::passed(), Some(error) => { @@ -2325,7 +2326,12 @@ async fn execute_replication_probe(result: &mut ReplicationCheckTargetStatus, op } } - if result.phases.put.status == "OK" && result.phases.version_fidelity.status == "OK" { + // DeleteMarker / VersionDelete address `probe_version_id`, the id the + // target actually assigned, so they run on a drifting target too. What + // they cannot prove there is the source-id addressing VersionFidelity + // already failed; what they do prove is that the ledger-addressed purge + // path the worker uses on such a target works against this endpoint. + if result.phases.put.status == "OK" && probe_version_id.is_some() { match operations.create_delete_marker(probe_version_id.as_deref()).await { Ok(version_id) => { delete_marker_version_id = version_id; @@ -3894,6 +3900,8 @@ mod tests { version_delete_error: Option<&'static str>, cleanup_error: Option<&'static str>, calls: Vec<&'static str>, + /// Version ids the delete-marker and version-delete phases addressed. + mutation_ids: Vec>, cleanup_ids: Vec>, } @@ -3938,16 +3946,18 @@ mod tests { } } - async fn create_delete_marker(&mut self, _version_id: Option<&str>) -> Result, S3ClientError> { + async fn create_delete_marker(&mut self, version_id: Option<&str>) -> Result, S3ClientError> { self.calls.push("delete-marker"); + self.mutation_ids.push(version_id.map(ToOwned::to_owned)); match self.delete_marker_error { Some(code) => Err(scripted_probe_error(code)), None => Ok(Some("marker-version".to_string())), } } - async fn delete_version(&mut self, _version_id: Option<&str>) -> Result<(), S3ClientError> { + async fn delete_version(&mut self, version_id: Option<&str>) -> Result<(), S3ClientError> { self.calls.push("version-delete"); + self.mutation_ids.push(version_id.map(ToOwned::to_owned)); match self.version_delete_error { Some(code) => Err(scripted_probe_error(code)), None => Ok(()), @@ -3968,12 +3978,12 @@ mod tests { } /// P1-19: a target that mints its own version ids must fail the - /// VersionFidelity phase with the machine-readable mismatch code, skip - /// the version-addressed mutation phases (they cannot mean anything on a - /// drifting target), and still clean up using the id the target actually - /// assigned — the source-derived id would never match. + /// VersionFidelity phase with the machine-readable mismatch code. The + /// mutation phases still run, addressing the id the target assigned + /// (the ledger the worker records per object, rustfs/backlog#2340), and + /// cleanup uses that id too — the source-derived id would never match. #[tokio::test] - async fn replication_probe_flags_version_minting_target() { + async fn replication_probe_flags_version_minting_target_and_probes_mutations_by_assigned_id() { let mut result = replication_check_target("arn:a", "OK", None); let mut operations = ScriptedReplicationProbe { minted_version_id: Some("target-minted-version"), @@ -3982,16 +3992,52 @@ mod tests { execute_replication_probe(&mut result, &mut operations).await; - assert_eq!(operations.calls, ["put", "cleanup"]); + assert_eq!(operations.calls, ["put", "delete-marker", "version-delete", "cleanup"]); assert_eq!(result.status, "FAILED"); assert_eq!(result.phases.put.status, "OK"); assert_eq!(result.phases.version_fidelity.status, "FAILED"); assert_eq!(result.phases.version_fidelity.code, Some(REPLICATION_CHECK_CODE_VERSION_MISMATCH)); - assert_eq!(result.phases.delete_marker.status, "SKIPPED"); - assert_eq!(result.phases.version_delete.status, "SKIPPED"); + assert_eq!(result.phases.delete_marker.status, "OK"); + assert_eq!(result.phases.version_delete.status, "OK"); + assert_eq!( + operations.mutation_ids, + [ + Some("target-minted-version".to_string()), + Some("target-minted-version".to_string()) + ], + "both mutation phases must address the id the target assigned" + ); assert_eq!(result.phases.ssec_passthrough.status, "SKIPPED"); assert_eq!(result.phases.cleanup.status, "OK"); - assert_eq!(operations.cleanup_ids, [Some("target-minted-version".to_string()), None, None, None]); + assert_eq!( + operations.cleanup_ids, + [ + Some("target-minted-version".to_string()), + None, + None, + Some("marker-version".to_string()) + ] + ); + } + + /// A drifting target that also refuses the ledger-addressed delete keeps + /// the phase-level evidence: VersionDelete fails on its own, apart from + /// the identity verdict. + #[tokio::test] + async fn replication_probe_reports_version_delete_failure_on_a_drifting_target() { + let mut result = replication_check_target("arn:a", "OK", None); + let mut operations = ScriptedReplicationProbe { + minted_version_id: Some("target-minted-version"), + version_delete_error: Some("AccessDenied"), + ..Default::default() + }; + + execute_replication_probe(&mut result, &mut operations).await; + + assert_eq!(result.phases.version_fidelity.status, "FAILED"); + assert_eq!(result.phases.delete_marker.status, "OK"); + assert_eq!(result.phases.version_delete.status, "FAILED"); + assert_eq!(result.phases.cleanup.status, "OK"); } #[tokio::test]