test: cover S3 error XML body compatibility (#2606)

crates/e2e_test/src/replication_extension_test.rs

@@ -190,6 +190,14 @@ async fn put_bucket_replication(
     Ok(())
 }
 
+async fn delete_bucket_replication(
+    env: &RustFSTestEnvironment,
+    bucket: &str,
+) -> Result<reqwest::Response, Box<dyn Error + Send + Sync>> {
+    let url = format!("{}/{bucket}?replication", env.url);
+    signed_request(http::Method::DELETE, &url, &env.access_key, &env.secret_key, None, None).await
+}
+
 async fn enable_bucket_versioning(env: &RustFSTestEnvironment, bucket: &str) -> Result<(), Box<dyn Error + Send + Sync>> {
     let client = env.create_s3_client();
     client
@@ -1084,6 +1092,54 @@ async fn test_remove_remote_target_rejects_target_used_by_replication() -> Resul
     Ok(())
 }
 
+#[tokio::test]
+#[serial]
+async fn test_delete_bucket_replication_removes_remote_target() -> Result<(), Box<dyn Error + Send + Sync>> {
+    init_logging();
+
+    let mut source_env = RustFSTestEnvironment::new().await?;
+    source_env.start_rustfs_server(vec![]).await?;
+
+    let mut target_env = RustFSTestEnvironment::new().await?;
+    target_env.start_rustfs_server_without_cleanup(vec![]).await?;
+
+    let source_bucket = "replication-delete-config-src";
+    let target_bucket = "replication-delete-config-dst";
+
+    let source_client = source_env.create_s3_client();
+    let target_client = target_env.create_s3_client();
+
+    source_client.create_bucket().bucket(source_bucket).send().await?;
+    target_client.create_bucket().bucket(target_bucket).send().await?;
+    enable_bucket_versioning(&source_env, source_bucket).await?;
+    enable_bucket_versioning(&target_env, target_bucket).await?;
+
+    let target_arn = set_replication_target(&source_env, source_bucket, &target_env, target_bucket).await?;
+    put_bucket_replication(&source_env, source_bucket, &target_arn).await?;
+
+    let delete_response = delete_bucket_replication(&source_env, source_bucket).await?;
+    assert!(
+        delete_response.status().is_success(),
+        "unexpected delete status: {}",
+        delete_response.status()
+    );
+
+    let targets_response = list_replication_targets_request(&source_env, Some(source_bucket)).await?;
+    assert_eq!(targets_response.status(), StatusCode::OK);
+    let targets: Vec<serde_json::Value> = targets_response.json().await?;
+    assert!(
+        targets
+            .iter()
+            .all(|target| target.get("arn").and_then(|arn| arn.as_str()) != Some(target_arn.as_str())),
+        "deleted replication config left stale target {target_arn}: {targets:?}"
+    );
+
+    let recreated_arn = set_replication_target(&source_env, source_bucket, &target_env, target_bucket).await?;
+    put_bucket_replication(&source_env, source_bucket, &recreated_arn).await?;
+
+    Ok(())
+}
+
 #[tokio::test]
 #[serial]
 async fn test_site_replication_resync_start_cancel_restart_real_dual_node() -> Result<(), Box<dyn Error + Send + Sync>> {

rustfs/src/app/bucket_usecase.rs

@@ -34,17 +34,19 @@ use http::StatusCode;
 use metrics::counter;
 use rustfs_config::RUSTFS_REGION;
 use rustfs_ecstore::bucket::{
+    bucket_target_sys::BucketTargetSys,
     lifecycle::bucket_lifecycle_ops::{
         enqueue_expiry_for_existing_objects, enqueue_transition_for_existing_objects, validate_transition_tier,
     },
     metadata::{
         BUCKET_CORS_CONFIG, BUCKET_LIFECYCLE_CONFIG, BUCKET_NOTIFICATION_CONFIG, BUCKET_POLICY_CONFIG,
         BUCKET_PUBLIC_ACCESS_BLOCK_CONFIG, BUCKET_REPLICATION_CONFIG, BUCKET_SSECONFIG, BUCKET_TAGGING_CONFIG,
-        BUCKET_VERSIONING_CONFIG,
+        BUCKET_TARGETS_FILE, BUCKET_VERSIONING_CONFIG,
     },
     metadata_sys,
     object_lock::ObjectLockApi,
     policy_sys::PolicySys,
+    target::BucketTargetType,
     utils::serialize,
     versioning::VersioningApi,
     versioning_sys::BucketVersioningSys,
@@ -99,6 +101,59 @@ fn sr_bucket_meta_item(bucket: String, item_type: &str) -> SRBucketMeta {
     }
 }
 
+fn replication_target_arns(config: &ReplicationConfiguration) -> HashSet<String> {
+    let mut arns = HashSet::new();
+
+    if !config.role.trim().is_empty() {
+        arns.insert(config.role.clone());
+        return arns;
+    }
+
+    for rule in &config.rules {
+        let arn = rule.destination.bucket.trim();
+        if !arn.is_empty() {
+            arns.insert(arn.to_string());
+        }
+    }
+
+    arns
+}
+
+async fn remove_replication_targets_for_config(bucket: &str, config: &ReplicationConfiguration) -> S3Result<()> {
+    let target_arns = replication_target_arns(config);
+    if target_arns.is_empty() {
+        return Ok(());
+    }
+
+    let mut targets = match metadata_sys::get_bucket_targets_config(bucket).await {
+        Ok(targets) => targets,
+        Err(StorageError::ConfigNotFound) => {
+            BucketTargetSys::get().update_all_targets(bucket, None).await;
+            return Ok(());
+        }
+        Err(err) => return Err(ApiError::from(err).into()),
+    };
+
+    let original_len = targets.targets.len();
+    targets.targets.retain(|target| {
+        target.target_type != BucketTargetType::ReplicationService || !target_arns.contains(target.arn.as_str())
+    });
+
+    if targets.targets.len() == original_len {
+        return Ok(());
+    }
+
+    let removed = original_len - targets.targets.len();
+    let json_targets = serde_json::to_vec(&targets).map_err(to_internal_error)?;
+    metadata_sys::update(bucket, BUCKET_TARGETS_FILE, json_targets)
+        .await
+        .map_err(ApiError::from)?;
+    BucketTargetSys::get().update_all_targets(bucket, Some(&targets)).await;
+    info!(bucket = %bucket, removed, "removed replication remote targets referenced by deleted bucket replication config");
+
+    Ok(())
+}
+
 fn versioning_configuration_has_object_lock_incompatible_settings(config: &VersioningConfiguration) -> bool {
     config.suspended()
         || config.exclude_folders.unwrap_or(false)
@@ -840,16 +895,26 @@ impl DefaultBucketUsecase {
             .get_bucket_info(&bucket, &BucketOptions::default())
             .await
             .map_err(ApiError::from)?;
+        let replication_config = match metadata_sys::get_replication_config(&bucket).await {
+            Ok((config, _)) => Some(config),
+            Err(StorageError::ConfigNotFound) => None,
+            Err(err) => return Err(ApiError::from(err).into()),
+        };
+
         metadata_sys::delete(&bucket, BUCKET_REPLICATION_CONFIG)
             .await
             .map_err(ApiError::from)?;
+        if let Some(config) = replication_config.as_ref()
+            && let Err(err) = remove_replication_targets_for_config(&bucket, config).await
+        {
+            warn!(bucket = %bucket, error = ?err, "failed to remove replication targets referenced by deleted bucket replication config");
+        }
 
         let item = sr_bucket_meta_item(bucket.clone(), "replication-config");
         if let Err(err) = site_replication_bucket_meta_hook(item).await {
             warn!(bucket = %bucket, error = ?err, "site replication bucket replication-config delete hook failed");
         }
 
-        // TODO: remove targets
         info!(bucket = %bucket, "deleted bucket replication config");
 
         Ok(S3Response::new(DeleteBucketReplicationOutput::default()))
@@ -1868,6 +1933,52 @@ mod tests {
         req
     }
 
+    fn replication_rule_for_target(arn: &str) -> ReplicationRule {
+        ReplicationRule {
+            delete_marker_replication: None,
+            delete_replication: None,
+            destination: Destination {
+                bucket: arn.to_string(),
+                ..Default::default()
+            },
+            existing_object_replication: None,
+            filter: None,
+            id: Some("rule-1".to_string()),
+            prefix: None,
+            priority: Some(1),
+            source_selection_criteria: None,
+            status: ReplicationRuleStatus::from_static(ReplicationRuleStatus::ENABLED),
+        }
+    }
+
+    #[test]
+    fn replication_target_arns_use_role_when_present() {
+        let role = "arn:rustfs:replication:us-east-1:source:bucket";
+        let destination = "arn:rustfs:replication:us-east-1:target:bucket";
+        let config = ReplicationConfiguration {
+            role: role.to_string(),
+            rules: vec![replication_rule_for_target(destination)],
+        };
+
+        let arns = replication_target_arns(&config);
+
+        assert!(arns.contains(role));
+        assert!(!arns.contains(destination));
+    }
+
+    #[test]
+    fn replication_target_arns_use_rule_destinations_without_role() {
+        let destination = "arn:rustfs:replication:us-east-1:target:bucket";
+        let config = ReplicationConfiguration {
+            role: String::new(),
+            rules: vec![replication_rule_for_target(destination)],
+        };
+
+        let arns = replication_target_arns(&config);
+
+        assert!(arns.contains(destination));
+    }
+
     #[test]
     fn versioning_configuration_has_object_lock_incompatible_settings_rejects_suspended() {
         let config = VersioningConfiguration {

rustfs/src/server/layer.rs

@@ -871,6 +871,30 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn test_fix_s3_error_message_in_xml_reports_changed_body() {
+        let body = Full::from(Bytes::from_static(b"<Error><Code>SignatureDoesNotMatch</Code></Error>"));
+
+        let (fixed, changed) = fix_s3_error_message_in_xml(body).await.unwrap();
+        let bytes = BodyExt::collect(fixed).await.unwrap().to_bytes();
+
+        assert!(changed);
+        assert!(bytes.starts_with(b"<Error><Code>SignatureDoesNotMatch</Code><Message>"));
+        assert!(bytes.ends_with(b"</Message></Error>"));
+    }
+
+    #[tokio::test]
+    async fn test_fix_s3_error_message_in_xml_reports_unchanged_body() {
+        let input = Bytes::from_static(b"<Error><Code>AccessDenied</Code></Error>");
+        let body = Full::from(input.clone());
+
+        let (fixed, changed) = fix_s3_error_message_in_xml(body).await.unwrap();
+        let bytes = BodyExt::collect(fixed).await.unwrap().to_bytes();
+
+        assert!(!changed);
+        assert_eq!(bytes, input);
+    }
+
     #[test]
     fn test_insert_missing_signature_error_message() {
         let (fixed, changed) =

scripts/s3-tests/excluded_tests.txt

@@ -8,11 +8,8 @@
 # - Intentionally unsupported by product decision (for example ACL authorization)
 
 # Vendor-specific / non-portable tests
-test_100_continue_error_retry
 test_account_usage
 test_atomic_conditional_write_1mb
-test_atomic_dual_conditional_write_1mb
-test_atomic_write_bucket_gone
 test_bucket_get_location
 test_bucket_head_extended
 test_bucket_header_acl_grants
@@ -128,8 +125,6 @@ test_bucket_policy_set_condition_operator_end_with_IfExists
 test_bucket_policy_upload_part_copy
 test_bucket_recreate_new_acl
 test_bucket_recreate_overwrite_acl
-test_copy_object_ifmatch_failed
-test_copy_object_ifnonematch_good
 test_cors_presigned_get_object_tenant_v2
 test_cors_presigned_get_object_v2
 test_cors_presigned_put_object_tenant_v2
@@ -171,7 +166,6 @@ test_lifecycle_cloud_transition_large_obj
 test_lifecycle_deletemarker_expiration
 test_lifecycle_deletemarker_expiration_with_days_tag
 test_lifecycle_expiration
-test_lifecycle_expiration_date
 test_lifecycle_expiration_days0
 test_lifecycle_expiration_header_put
 test_lifecycle_expiration_header_head
@@ -180,10 +174,8 @@ test_lifecycle_expiration_newer_noncurrent
 test_lifecycle_expiration_noncur_tags1
 test_lifecycle_expiration_size_gt
 test_lifecycle_expiration_size_lt
-test_lifecycle_expiration_tags1
 test_lifecycle_expiration_tags2
 test_lifecycle_expiration_versioned_tags2
-test_lifecycle_expiration_versioning_enabled
 test_lifecycle_multipart_expiration
 test_lifecycle_noncur_cloud_transition
 test_lifecycle_noncur_expiration
@@ -273,22 +265,13 @@ test_access_bucket_publicreadwrite_object_publicreadwrite
 test_object_anon_put_write_access
 test_get_public_acl_bucket_policy_status
 test_get_authpublic_acl_bucket_policy_status
-test_get_publicpolicy_acl_bucket_policy_status
-test_get_nonpublicpolicy_acl_bucket_policy_status
 test_block_public_put_bucket_acls
 test_block_public_object_canned_acls
 test_ignore_public_acls
-test_bucket_policy_acl
-test_bucketv2_policy_acl
-test_bucket_policy_put_obj_acl
-test_object_presigned_put_object_with_acl
-test_object_put_acl_mtime
 test_versioned_object_acl
-test_object_presigned_put_object_with_acl_tenant
 test_bucket_acl_canned
 test_bucket_acl_canned_authenticatedread
 test_bucket_acl_canned_during_create
-test_bucket_acl_canned_private_to_private
 test_bucket_acl_canned_publicreadwrite
 test_bucket_acl_default
 test_bucket_acl_grant_email
@@ -300,7 +283,6 @@ test_bucket_acl_grant_userid_readacp
 test_bucket_acl_grant_userid_write
 test_bucket_acl_grant_userid_writeacp
 test_bucket_acl_revoke_all
-test_bucket_concurrent_set_canned_acl
 test_object_acl
 test_object_acl_canned
 test_object_acl_canned_authenticatedread
@@ -316,9 +298,4 @@ test_object_acl_readacp
 test_object_acl_write
 test_object_acl_writeacp
 test_put_bucket_acl_grant_group_read
-test_object_raw_authenticated_bucket_acl
-test_object_raw_authenticated_object_acl
 test_object_raw_get_bucket_acl
-test_object_raw_get_object_acl
-test_cors_presigned_put_object_with_acl
-test_cors_presigned_put_object_tenant_with_acl

scripts/s3-tests/implemented_tests.txt

@@ -508,3 +508,28 @@ test_versioning_obj_create_overwrite_multipart
 test_versioning_obj_suspended_copy
 test_rm_bucket_logging
 test_versioned_concurrent_object_create_concurrent_remove
+
+# Reclassified from excluded/unimplemented candidates
+test_100_continue_error_retry
+test_atomic_dual_conditional_write_1mb
+test_atomic_write_bucket_gone
+test_bucket_acl_canned_private_to_private
+test_bucket_concurrent_set_canned_acl
+test_bucket_policy_acl
+test_bucket_policy_put_obj_acl
+test_bucketv2_policy_acl
+test_copy_object_ifmatch_failed
+test_copy_object_ifnonematch_good
+test_cors_presigned_put_object_tenant_with_acl
+test_cors_presigned_put_object_with_acl
+test_get_nonpublicpolicy_acl_bucket_policy_status
+test_get_publicpolicy_acl_bucket_policy_status
+test_lifecycle_expiration_date
+test_lifecycle_expiration_tags1
+test_lifecycle_expiration_versioning_enabled
+test_object_presigned_put_object_with_acl
+test_object_presigned_put_object_with_acl_tenant
+test_object_put_acl_mtime
+test_object_raw_authenticated_bucket_acl
+test_object_raw_authenticated_object_acl
+test_object_raw_get_object_acl

scripts/s3-tests/non_standard_tests.txt

@@ -14,11 +14,8 @@
 # - SSE-KMS: Ceph-specific KMS extensions
 # - Error format differences: Minor response format variations
 
-test_100_continue_error_retry
 test_account_usage
 test_atomic_conditional_write_1mb
-test_atomic_dual_conditional_write_1mb
-test_atomic_write_bucket_gone
 test_bucket_get_location
 test_bucket_head_extended
 test_bucket_header_acl_grants
@@ -134,8 +131,6 @@ test_bucket_policy_set_condition_operator_end_with_IfExists
 test_bucket_policy_upload_part_copy
 test_bucket_recreate_new_acl
 test_bucket_recreate_overwrite_acl
-test_copy_object_ifmatch_failed
-test_copy_object_ifnonematch_good
 test_cors_presigned_get_object_tenant_v2
 test_cors_presigned_get_object_v2
 test_cors_presigned_put_object_tenant_v2
@@ -177,7 +172,6 @@ test_lifecycle_cloud_transition_large_obj
 test_lifecycle_deletemarker_expiration
 test_lifecycle_deletemarker_expiration_with_days_tag
 test_lifecycle_expiration
-test_lifecycle_expiration_date
 test_lifecycle_expiration_days0
 test_lifecycle_expiration_header_put
 test_lifecycle_expiration_header_head
@@ -186,10 +180,8 @@ test_lifecycle_expiration_newer_noncurrent
 test_lifecycle_expiration_noncur_tags1
 test_lifecycle_expiration_size_gt
 test_lifecycle_expiration_size_lt
-test_lifecycle_expiration_tags1
 test_lifecycle_expiration_tags2
 test_lifecycle_expiration_versioned_tags2
-test_lifecycle_expiration_versioning_enabled
 test_lifecycle_multipart_expiration
 test_lifecycle_noncur_cloud_transition
 test_lifecycle_noncur_expiration