diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 940d0c7d6..89adaf029 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,17 +5,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Current Context - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) -- Branch: `overtrue/arch-rustfs-local-compat-consumers` -- Baseline: completed `C-011/C-012/C-013/API-055/API-059/API-079/API-080/API-081/API-082/API-083/API-084/API-085/API-086/API-087/API-088/API-089/API-090/API-091/API-092/API-093/API-094/API-095/API-096/API-097/API-098/API-099/API-100/API-101/API-102/API-103/API-104/API-105/API-106/API-107/API-108/API-109/API-110`. -- Based on: API-109 slice. +- Branch: `overtrue/arch-storage-rpc-s3-local-compat-consumers` +- Baseline: completed `C-011/C-012/C-013/API-055/API-059/API-079/API-080/API-081/API-082/API-083/API-084/API-085/API-086/API-087/API-088/API-089/API-090/API-091/API-092/API-093/API-094/API-095/API-096/API-097/API-098/API-099/API-100/API-101/API-102/API-103/API-104/API-105/API-106/API-107/API-108/API-109/API-110/API-111`. +- Based on: API-110 slice. - PR type for this branch: `pure-move` - Runtime behavior changes: none. -- Rust code changes: collapse crate-qualified RustFS app/admin/storage local +- Rust code changes: collapse crate-qualified storage RPC and S3 API local compatibility consumer paths into relative `super::`/`super::super::` paths. -- CI/script changes: guard selected RustFS app/admin/storage local consumers +- CI/script changes: guard selected storage RPC and S3 API local consumers against crate-qualified compatibility paths. -- Docs changes: record the API-110 relative local compatibility consumer - cleanup. +- Docs changes: record the API-111 relative storage RPC/S3 API local + compatibility consumer cleanup. ## Phase 0 Tasks @@ -571,6 +571,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block - Verification: RustFS test-target compile coverage, local compatibility consumer residual scan, migration and layer guards, formatting, diff hygiene, Rust risk scan, pre-commit quality gate, and three-expert review. +- [x] `API-111` Collapse storage RPC and S3 API local compatibility consumers. + - Completed slice: replace crate-qualified storage RPC and S3 API local + compatibility consumers with relative owner paths. + - Acceptance: selected storage RPC and S3 API modules no longer point back to + local compatibility facades through crate-qualified paths; migration rules + reject regressions. + - Must preserve: internode RPC request handling, node service helper tests, + S3 list bucket output mapping, multipart listing output mapping, and ETag + helper behavior. + - Verification: RustFS test-target compile coverage, storage RPC/S3 API + local compatibility consumer residual scan, migration and layer guards, + formatting, diff hygiene, Rust risk scan, pre-commit quality gate, and + three-expert review. - [x] `G-012` Inventory placement and repair invariants. - Acceptance: [`placement-repair-invariants.md`](placement-repair-invariants.md) records @@ -3604,14 +3617,26 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | pass | API-110 keeps selected app/admin/storage local compatibility consumers owner-relative by replacing crate-qualified compatibility paths with scoped `super::` or `super::super::` paths. | -| Migration preservation | pass | The new guard rejects crate-qualified app/admin/storage local compatibility consumer paths while preserving the same facade names and aliases. | -| Testing/verification | pass | Focused compile, local compatibility consumer residual scan, migration guard, layer guard, formatting, diff hygiene, risk scan, and full pre-commit passed. | +| Quality/architecture | pass | API-111 keeps selected storage RPC and S3 API local compatibility consumers owner-relative by replacing crate-qualified compatibility paths with scoped `super::` or `super::super::` paths. | +| Migration preservation | pass | The new guard rejects crate-qualified storage RPC/S3 API local compatibility consumer paths while preserving the same facade names and aliases. | +| Testing/verification | pass | Focused compile, storage RPC/S3 API local compatibility consumer residual scan, migration guard, layer guard, formatting, diff hygiene, risk scan, and full pre-commit passed. | ## Verification Notes Passed before push: +- Issue #660 API-111 current slice: + - `cargo check -p rustfs --tests`: passed. + - `cargo fmt --all`: passed. + - `cargo fmt --all --check`: passed. + - `git diff --check`: passed. + - `bash -n scripts/check_architecture_migration_rules.sh`: passed. + - `./scripts/check_architecture_migration_rules.sh`: passed. + - `./scripts/check_layer_dependencies.sh`: passed. + - Storage RPC/S3 API local compatibility consumer residual scan: passed. + - Rust risk scan on changed Rust files and guard script: passed. + - `make pre-commit`: passed. + - Issue #660 API-110 current slice: - `cargo check -p rustfs --tests`: passed. - `cargo fmt --all`: passed. diff --git a/rustfs/src/storage/rpc/http_service.rs b/rustfs/src/storage/rpc/http_service.rs index 43636def8..f97d89070 100644 --- a/rustfs/src/storage/rpc/http_service.rs +++ b/rustfs/src/storage/rpc/http_service.rs @@ -12,13 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::storage_compat::DEFAULT_READ_BUFFER_SIZE; +use super::storage_compat::StorageDiskRpcExt as _; +use super::storage_compat::WalkDirOptions; +use super::storage_compat::find_local_disk_by_ref; +use super::storage_compat::verify_rpc_signature; use crate::server::RPC_PREFIX; use crate::storage::request_context::spawn_traced; -use crate::storage::rpc::storage_compat::DEFAULT_READ_BUFFER_SIZE; -use crate::storage::rpc::storage_compat::StorageDiskRpcExt as _; -use crate::storage::rpc::storage_compat::WalkDirOptions; -use crate::storage::rpc::storage_compat::find_local_disk_by_ref; -use crate::storage::rpc::storage_compat::verify_rpc_signature; use bytes::{Bytes, BytesMut}; use futures_util::TryStreamExt; use http::{HeaderMap, Method, Request, Response, StatusCode, Uri}; diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index ad9439d06..2a39687a2 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -12,17 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::admin::service::{ - config::{reload_dynamic_config_runtime_state, reload_runtime_config_snapshot}, - site_replication::reload_site_replication_runtime_state, -}; -use crate::storage::rpc::storage_compat::{ +use super::storage_compat::{ CollectMetricsOpts, DeleteOptions, DiskError, DiskInfoOptions, DiskStore, FileInfoVersions, LocalPeerS3Client, MetricType, PEER_RESTSIGNAL, PEER_RESTSUB_SYS, ReadMultipleReq, ReadMultipleResp, ReadOptions, SERVICE_SIGNAL_REFRESH_CONFIG, SERVICE_SIGNAL_RELOAD_DYNAMIC, StorageDiskRpcExt as _, StoragePeerS3ClientExt as _, UpdateMetadataOpts, all_local_disk_path, collect_local_metrics, find_local_disk_by_ref, get_global_lock_client, get_local_server_property, load_bucket_metadata, reload_transition_tier_config, resolve_object_store_handle, set_bucket_metadata, }; +use crate::admin::service::{ + config::{reload_dynamic_config_runtime_state, reload_runtime_config_snapshot}, + site_replication::reload_site_replication_runtime_state, +}; use bytes::Bytes; use futures::Stream; use futures_util::future::join_all; @@ -122,11 +122,11 @@ fn unimplemented_rpc(method: &str) -> Status { Status::unimplemented(format!("{method} is not implemented")) } -fn background_rebalance_start_error_message(result: crate::storage::rpc::storage_compat::Result<()>) -> Option { +fn background_rebalance_start_error_message(result: super::storage_compat::Result<()>) -> Option { result.err().map(|err| format!("start_rebalance failed: {err}")) } -fn stop_rebalance_response(result: crate::storage::rpc::storage_compat::Result<()>) -> StopRebalanceResponse { +fn stop_rebalance_response(result: super::storage_compat::Result<()>) -> StopRebalanceResponse { match result { Ok(_) => StopRebalanceResponse { success: true, @@ -2389,7 +2389,7 @@ mod tests { #[test] fn test_background_rebalance_start_error_message_formats_error() { - let message = background_rebalance_start_error_message(Err(crate::storage::rpc::storage_compat::Error::other("boom"))) + let message = background_rebalance_start_error_message(Err(super::super::storage_compat::Error::other("boom"))) .expect("background rebalance start failure should be formatted"); assert!(message.contains("start_rebalance failed")); @@ -2398,7 +2398,7 @@ mod tests { #[test] fn test_stop_rebalance_response_reports_local_stop_error() { - let response = stop_rebalance_response(Err(crate::storage::rpc::storage_compat::Error::other("boom"))); + let response = stop_rebalance_response(Err(super::super::storage_compat::Error::other("boom"))); assert!(!response.success); assert!(response.error_info.as_deref().is_some_and(|message| message.contains("boom"))); @@ -2736,7 +2736,7 @@ mod tests { vars.insert(PEER_RESTSIGNAL.to_string(), SERVICE_SIGNAL_RELOAD_DYNAMIC.to_string()); vars.insert( PEER_RESTSUB_SYS.to_string(), - crate::storage::rpc::storage_compat::STORAGE_CLASS_SUB_SYS.to_string(), + super::super::storage_compat::STORAGE_CLASS_SUB_SYS.to_string(), ); let request = Request::new(SignalServiceRequest { diff --git a/rustfs/src/storage/s3_api/bucket.rs b/rustfs/src/storage/s3_api/bucket.rs index 69e3cfedc..cde560e1a 100644 --- a/rustfs/src/storage/s3_api/bucket.rs +++ b/rustfs/src/storage/s3_api/bucket.rs @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::storage_compat::to_s3s_etag; use crate::storage::s3_api::common::rustfs_owner; -use crate::storage::s3_api::storage_compat::to_s3s_etag; use percent_encoding::percent_decode_str; use rustfs_storage_api::{ BucketInfo, ListObjectVersionsInfo as StorageListObjectVersionsInfo, ListObjectsV2Info as StorageListObjectsV2Info, diff --git a/rustfs/src/storage/s3_api/multipart.rs b/rustfs/src/storage/s3_api/multipart.rs index dae7401ac..c51b8957e 100644 --- a/rustfs/src/storage/s3_api/multipart.rs +++ b/rustfs/src/storage/s3_api/multipart.rs @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::storage_compat::to_s3s_etag; use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner}; -use crate::storage::s3_api::storage_compat::to_s3s_etag; use rustfs_storage_api::{ListMultipartsInfo, ListPartsInfo}; use s3s::dto::{CommonPrefix, ListMultipartUploadsOutput, ListPartsOutput, MultipartUpload, Part, Timestamp}; use s3s::{S3Error, S3ErrorCode}; @@ -186,12 +186,12 @@ pub(crate) fn build_list_multipart_uploads_output( #[cfg(test)] mod tests { + use super::super::storage_compat::to_s3s_etag; use super::{ MAX_MULTIPART_UPLOADS_LIST, build_list_multipart_uploads_output, build_list_parts_output, parse_list_multipart_uploads_params, parse_list_parts_params, parse_upload_part_number, }; use crate::storage::s3_api::common::{rustfs_initiator, rustfs_owner}; - use crate::storage::s3_api::storage_compat::to_s3s_etag; use rustfs_storage_api::{ListMultipartsInfo, ListPartsInfo, MultipartInfo, PartInfo}; use s3s::S3ErrorCode; use s3s::dto::Timestamp; diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index b118ab190..a1d857089 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -102,6 +102,7 @@ RUSTFS_LOCAL_COMPAT_OWNER_SELF_PATH_HITS_FILE="${TMP_DIR}/rustfs_local_compat_ow RUSTFS_ROOT_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_root_compat_relative_consumer_hits.txt" RUSTFS_STORAGE_CORE_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_storage_core_compat_relative_consumer_hits.txt" RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_local_compat_relative_consumer_hits.txt" +RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_storage_local_compat_relative_consumer_hits.txt" SCANNER_BUCKET_STORAGE_COMPAT_MODULE_HITS_FILE="${TMP_DIR}/scanner_bucket_storage_compat_module_hits.txt" NOTIFY_STORAGE_COMPAT_MODULE_HITS_FILE="${TMP_DIR}/notify_storage_compat_module_hits.txt" OBS_STORAGE_COMPAT_PASSTHROUGH_HITS_FILE="${TMP_DIR}/obs_storage_compat_passthrough_hits.txt" @@ -1099,6 +1100,22 @@ if [[ -s "$RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE" ]]; then report_failure "RustFS local compatibility consumers must use relative owner paths instead of crate-qualified local compatibility paths: $(paste -sd '; ' "$RUSTFS_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE")" fi +( + cd "$ROOT_DIR" + { + rg -n --with-filename 'crate::storage::rpc::storage_compat' \ + rustfs/src/storage/rpc/http_service.rs \ + rustfs/src/storage/rpc/node_service.rs || true + rg -n --with-filename 'crate::storage::s3_api::storage_compat' \ + rustfs/src/storage/s3_api/bucket.rs \ + rustfs/src/storage/s3_api/multipart.rs || true + } +) >"$RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE" + +if [[ -s "$RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE" ]]; then + report_failure "RustFS storage RPC/S3 API compatibility consumers must use relative owner paths instead of crate-qualified local compatibility paths: $(paste -sd '; ' "$RUSTFS_STORAGE_LOCAL_COMPAT_RELATIVE_CONSUMER_HITS_FILE")" +fi + ( cd "$ROOT_DIR" rg -n --no-heading 'pub\(crate\)\s+use rustfs_ecstore::api::bucket::\{[^}]*\b(?:bucket_target_sys|lifecycle|metadata_sys|replication|versioning|versioning_sys)\b[^}]*\}\s*;' \