From b94e7d514de132576447fd8cab999c316d3bc586 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Wed, 1 Jul 2026 22:18:06 +0800 Subject: [PATCH] refactor(admin): hide replication resync DTOs (#4146) --- .../architecture/ecstore-module-split-plan.md | 3 +++ rustfs/src/admin/handlers/site_replication.rs | 16 +++--------- rustfs/src/admin/router.rs | 21 +++++++-------- rustfs/src/admin/storage_api.rs | 26 ++++++++++++++++--- scripts/check_architecture_migration_rules.sh | 23 ++++++++++++++++ 5 files changed, 60 insertions(+), 29 deletions(-) diff --git a/docs/architecture/ecstore-module-split-plan.md b/docs/architecture/ecstore-module-split-plan.md index 16ca88608..125bf4c06 100644 --- a/docs/architecture/ecstore-module-split-plan.md +++ b/docs/architecture/ecstore-module-split-plan.md @@ -120,6 +120,9 @@ Current coupling: - bucket metadata migration and bucket target removal checks use local replication bridges instead of importing resyncer codec or config helper internals; +- admin replication extension target filtering and resync request construction + stay behind the admin storage boundary instead of exposing replication work + DTO construction to handlers; - global replication pool/stat initialization still lives with ECStore runtime compatibility state; - modules inside `bucket/replication` use local relative paths rather than the diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 27d077e6c..4cfcb37c6 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -28,7 +28,7 @@ use crate::admin::storage_api::bucket::metadata::{ BUCKET_SSECONFIG, BUCKET_TAGGING_CONFIG, BUCKET_TARGETS_FILE, BUCKET_VERSIONING_CONFIG, OBJECT_LOCK_CONFIG, }; use crate::admin::storage_api::bucket::metadata_sys; -use crate::admin::storage_api::bucket::replication::ResyncOpts; +use crate::admin::storage_api::bucket::replication; use crate::admin::storage_api::bucket::target::{ARN, BucketTarget, BucketTargetType, BucketTargets, Credentials}; use crate::admin::storage_api::bucket::target_sys::BucketTargetSys; use crate::admin::storage_api::bucket::utils::{deserialize, serialize}; @@ -4289,12 +4289,7 @@ async fn start_site_bucket_resync(bucket: &str, peer: &PeerInfo, resync_id: &str }; if let Err(err) = pool - .start_bucket_resync(ResyncOpts { - bucket: bucket.to_string(), - arn: target_arn, - resync_id: resync_id.to_string(), - resync_before: reset_before, - }) + .start_bucket_resync(replication::resync_opts(bucket, target_arn, resync_id, reset_before)) .await { bucket_status.status = "failed".to_string(); @@ -4357,12 +4352,7 @@ async fn cancel_site_bucket_resync(bucket: &str, peer: &PeerInfo, resync_id: &st }; if let Err(err) = pool - .cancel_bucket_resync(ResyncOpts { - bucket: bucket.to_string(), - arn: target_arn, - resync_id: resync_id.to_string(), - resync_before: None, - }) + .cancel_bucket_resync(replication::resync_opts(bucket, target_arn, resync_id, None)) .await { bucket_status.status = "failed".to_string(); diff --git a/rustfs/src/admin/router.rs b/rustfs/src/admin/router.rs index bac70e1c3..c1563dac5 100644 --- a/rustfs/src/admin/router.rs +++ b/rustfs/src/admin/router.rs @@ -15,7 +15,7 @@ use super::storage_api::bucket::bandwidth::monitor::BandwidthDetails; use super::storage_api::bucket::metadata::BUCKET_TARGETS_FILE; use super::storage_api::bucket::metadata_sys; -use super::storage_api::bucket::replication::{BucketReplicationResyncStatus, BucketStats, ObjectOpts, ResyncOpts}; +use super::storage_api::bucket::replication::{self, BucketReplicationResyncStatus, BucketStats}; use super::storage_api::bucket::target::{BucketTarget, BucketTargetType, BucketTargets}; use super::storage_api::bucket::target_sys::{ BucketTargetSys, PutObjectOptions, RemoveObjectOptions, S3ClientError, TargetClient, @@ -59,7 +59,7 @@ use rustfs_config::{ ENABLE_KEY, WEBHOOK_AUTH_TOKEN, WEBHOOK_CLIENT_CA, WEBHOOK_CLIENT_CERT, WEBHOOK_CLIENT_KEY, WEBHOOK_ENDPOINT, WEBHOOK_SKIP_TLS_VERIFY, }; -use rustfs_filemeta::{ReplicationStatusType, ReplicationType}; +use rustfs_filemeta::ReplicationStatusType; use rustfs_madmin::utils::parse_duration; use rustfs_notify::{Event as NotificationEvent, notification_system}; use rustfs_policy::policy::action::{Action, S3Action}; @@ -1807,10 +1807,7 @@ fn validate_replication_check_config_targets( fn filter_replication_check_targets(targets: BucketTargets, config: &s3s::dto::ReplicationConfiguration) -> Vec { let referenced_arns = config - .filter_target_arns(&ObjectOpts { - op_type: ReplicationType::All, - ..Default::default() - }) + .filter_all_replication_target_arns() .into_iter() .collect::>(); @@ -2158,12 +2155,12 @@ async fn start_replication_resync(bucket: &str, reset: &ReplicationResetStartReq return Err(s3_error!(InternalError, "replication pool is not initialized")); }; - pool.start_bucket_resync(ResyncOpts { - bucket: bucket.to_string(), - arn: resolved_arn.clone(), - resync_id: reset.reset_id.clone(), - resync_before: reset.reset_before, - }) + pool.start_bucket_resync(replication::resync_opts( + bucket, + resolved_arn.clone(), + &reset.reset_id, + reset.reset_before, + )) .await .map_err(|e| s3_error!(InternalError, "{e}"))?; diff --git a/rustfs/src/admin/storage_api.rs b/rustfs/src/admin/storage_api.rs index b75a95272..62459a9e1 100644 --- a/rustfs/src/admin/storage_api.rs +++ b/rustfs/src/admin/storage_api.rs @@ -16,6 +16,7 @@ use std::ops::Deref; use std::sync::Arc; use rustfs_storage_api as storage_contracts; +use time::OffsetDateTime; mod ecstore_bucket { pub(crate) use crate::storage::storage_api::ecstore_bucket::{ @@ -142,14 +143,18 @@ pub(crate) fn encode_rebalance_stop_propagation_record(record: &RebalanceStopPro } pub(crate) trait AdminReplicationConfigExt { - fn filter_target_arns(&self, obj: &replication::ObjectOpts) -> Vec; + fn filter_all_replication_target_arns(&self) -> Vec; fn has_existing_object_replication(&self, arn: &str) -> (bool, bool); } impl AdminReplicationConfigExt for s3s::dto::ReplicationConfiguration { - fn filter_target_arns(&self, obj: &replication::ObjectOpts) -> Vec { + fn filter_all_replication_target_arns(&self) -> Vec { + let obj = ecstore_bucket::replication::ObjectOpts { + op_type: rustfs_filemeta::ReplicationType::All, + ..Default::default() + }; ::filter_target_arns( - self, obj, + self, &obj, ) } @@ -304,12 +309,25 @@ pub(crate) mod quota { pub(crate) mod replication { pub(crate) type BucketReplicationResyncStatus = super::ecstore_bucket::replication::BucketReplicationResyncStatus; pub(crate) type BucketStats = super::ecstore_bucket::replication::BucketStats; - pub(crate) type ObjectOpts = super::ecstore_bucket::replication::ObjectOpts; pub(crate) type ResyncOpts = super::ecstore_bucket::replication::ResyncOpts; #[cfg(test)] pub(crate) type ResyncStatusType = super::ecstore_bucket::replication::ResyncStatusType; #[cfg(test)] pub(crate) type TargetReplicationResyncStatus = super::ecstore_bucket::replication::TargetReplicationResyncStatus; + + pub(crate) fn resync_opts( + bucket: &str, + arn: String, + resync_id: &str, + resync_before: Option, + ) -> ResyncOpts { + ResyncOpts { + bucket: bucket.to_string(), + arn, + resync_id: resync_id.to_string(), + resync_before, + } + } } pub(crate) mod target { diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index 44f43ed63..2e1cd8546 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -199,6 +199,7 @@ FUZZ_ECSTORE_COMPAT_BYPASS_HITS_FILE="${TMP_DIR}/fuzz_ecstore_compat_bypass_hits EXTERNAL_ECSTORE_API_BOUNDARY_HITS_FILE="${TMP_DIR}/external_ecstore_api_boundary_hits.txt" REPLICATION_FACADE_BYPASS_HITS_FILE="${TMP_DIR}/replication_facade_bypass_hits.txt" REPLICATION_FACADE_WILDCARD_EXPORT_HITS_FILE="${TMP_DIR}/replication_facade_wildcard_export_hits.txt" +ADMIN_REPLICATION_DTO_BOUNDARY_BYPASS_HITS_FILE="${TMP_DIR}/admin_replication_dto_boundary_bypass_hits.txt" REPLICATION_BANDWIDTH_BOUNDARY_BYPASS_HITS_FILE="${TMP_DIR}/replication_bandwidth_boundary_bypass_hits.txt" REPLICATION_CONFIG_STORE_BYPASS_HITS_FILE="${TMP_DIR}/replication_config_store_bypass_hits.txt" REPLICATION_ERROR_BOUNDARY_BYPASS_HITS_FILE="${TMP_DIR}/replication_error_boundary_bypass_hits.txt" @@ -2517,6 +2518,28 @@ if [[ -s "$REPLICATION_FACADE_WILDCARD_EXPORT_HITS_FILE" ]]; then report_failure "replication facade must use explicit compatibility exports instead of wildcard re-exports: $(paste -sd '; ' "$REPLICATION_FACADE_WILDCARD_EXPORT_HITS_FILE")" fi +( + cd "$ROOT_DIR" + { + rg -n --with-filename '\b(ObjectOpts|ResyncOpts)\s*\{' \ + rustfs/src/admin \ + --glob '*.rs' \ + --glob '!rustfs/src/admin/storage_api.rs' || true + rg -n -U --with-filename 'use\s+(?:crate::admin::storage_api::bucket::replication|super::storage_api::bucket::replication)::\{[^}]*\b(ObjectOpts|ResyncOpts)\b' \ + rustfs/src/admin \ + --glob '*.rs' \ + --glob '!rustfs/src/admin/storage_api.rs' || true + rg -n --with-filename '(?:crate::admin::storage_api::bucket::replication|super::storage_api::bucket::replication|replication)::(?:ObjectOpts|ResyncOpts)\b' \ + rustfs/src/admin \ + --glob '*.rs' \ + --glob '!rustfs/src/admin/storage_api.rs' || true + } +) >"$ADMIN_REPLICATION_DTO_BOUNDARY_BYPASS_HITS_FILE" + +if [[ -s "$ADMIN_REPLICATION_DTO_BOUNDARY_BYPASS_HITS_FILE" ]]; then + report_failure "admin replication ObjectOpts/ResyncOpts construction must stay behind rustfs/src/admin/storage_api.rs: $(paste -sd '; ' "$ADMIN_REPLICATION_DTO_BOUNDARY_BYPASS_HITS_FILE")" +fi + ( cd "$ROOT_DIR" {