From cd26594673ce780f4fc1a35fa68a480264671c6f Mon Sep 17 00:00:00 2001 From: Mathew Storm Date: Mon, 8 Jun 2026 16:37:11 -0400 Subject: [PATCH 1/2] fix(s3): treat NoSuchKey as success in bulk DeleteObjects --- src/api/s3/delete.rs | 17 +++++++++++++++-- src/api/s3/xml.rs | 19 +++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/api/s3/delete.rs b/src/api/s3/delete.rs index 43824a04..08ff1460 100644 --- a/src/api/s3/delete.rs +++ b/src/api/s3/delete.rs @@ -83,8 +83,21 @@ pub async fn handle_delete_objects( } ret_deleted.push(s3_xml::Deleted { key: s3_xml::Value(obj.key.clone()), - version_id: s3_xml::Value(hex::encode(deleted_version)), - delete_marker_version_id: s3_xml::Value(hex::encode(delete_marker_version)), + version_id: Some(s3_xml::Value(hex::encode(deleted_version))), + delete_marker_version_id: Some(s3_xml::Value(hex::encode( + delete_marker_version, + ))), + }); + } + Err(Error::NoSuchKey) => { + if cmd.quiet { + continue; + } + // Deleting a non-existent key is a success in S3 + ret_deleted.push(s3_xml::Deleted { + key: s3_xml::Value(obj.key.clone()), + version_id: None, + delete_marker_version_id: None, }); } Err(e) => { diff --git a/src/api/s3/xml.rs b/src/api/s3/xml.rs index 5970c964..8a99c2fb 100644 --- a/src/api/s3/xml.rs +++ b/src/api/s3/xml.rs @@ -44,10 +44,13 @@ pub struct LocationConstraint { pub struct Deleted { #[serde(rename = "Key")] pub key: Value, - #[serde(rename = "VersionId")] - pub version_id: Value, - #[serde(rename = "DeleteMarkerVersionId")] - pub delete_marker_version_id: Value, + #[serde(rename = "VersionId", skip_serializing_if = "Option::is_none")] + pub version_id: Option, + #[serde( + rename = "DeleteMarkerVersionId", + skip_serializing_if = "Option::is_none" + )] + pub delete_marker_version_id: Option, } #[derive(Debug, Serialize, PartialEq, Eq)] @@ -497,13 +500,13 @@ mod tests { deleted: vec![ Deleted { key: Value("a/plop".to_string()), - version_id: Value("qsdfjklm".to_string()), - delete_marker_version_id: Value("wxcvbn".to_string()), + version_id: Some(Value("qsdfjklm".to_string())), + delete_marker_version_id: Some(Value("wxcvbn".to_string())), }, Deleted { key: Value("b/plip".to_string()), - version_id: Value("1234".to_string()), - delete_marker_version_id: Value("4321".to_string()), + version_id: Some(Value("1234".to_string())), + delete_marker_version_id: Some(Value("4321".to_string())), }, ], errors: vec![ From 8eb7628bf4b3b919b62ec3119e35da8f06ce1d12 Mon Sep 17 00:00:00 2001 From: Mathew Storm Date: Mon, 8 Jun 2026 17:02:37 -0400 Subject: [PATCH 2/2] test(s3): cover bulk DeleteObjects of a non-existent key --- src/garage/tests/s3/objects.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/garage/tests/s3/objects.rs b/src/garage/tests/s3/objects.rs index 53e8231d..71dd3c95 100644 --- a/src/garage/tests/s3/objects.rs +++ b/src/garage/tests/s3/objects.rs @@ -554,7 +554,7 @@ async fn test_deleteobject() { assert!(l.contents.is_none()); - // Deleting a non-existing object shouldn't be a problem + // Deleting a non-existing object shouldn't be a problem... ctx.client .delete_object() .bucket(&bucket) @@ -562,4 +562,27 @@ async fn test_deleteobject() { .send() .await .unwrap(); + + // ...and bulk-deleting a non-existing object shouldn't be either + let r = ctx + .client + .delete_objects() + .bucket(&bucket) + .delete( + Delete::builder() + .objects( + ObjectIdentifier::builder() + .key("does-not-exist") + .build() + .unwrap(), + ) + .build() + .unwrap(), + ) + .send() + .await + .unwrap(); + + assert_eq!(r.deleted.unwrap().len(), 1); // reported as deleted... + assert!(r.errors.unwrap_or_default().is_empty()); // ...not as an error }