From 741b4dab7fb4575e8263be754dbda68a155f50eb Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 26 Jun 2026 23:20:25 +0800 Subject: [PATCH] refactor: close external storage API boundary guard (#3925) --- docs/architecture/crate-boundaries.md | 5 ++ docs/architecture/migration-progress.md | 61 +++++++++++++------ scripts/check_architecture_migration_rules.sh | 23 +++++++ 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index 39f35749a..6bec05422 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -215,6 +215,11 @@ Test and fuzz `storage_compat.rs` harnesses must also stay narrow. Heal and scanner test harnesses must expose ECStore contracts through direct aliases or local wrappers, and fuzz harnesses must wrap bucket utility entrypoints instead of restoring grouped ECStore passthrough exports. +External ECStore API facade imports must stay inside local `storage_api` +boundary files after the external runtime, test, and fuzz consumers have been +narrowed. IAM, heal, scanner, notify, observability, Swift, S3 Select, e2e, and +fuzz code must not reintroduce direct `rustfs_ecstore::api::...` references +outside those boundary files. ECStore ClusterControlPlane read models must stay owned by the crate-private `cluster` module. Public access goes through `rustfs_ecstore::api::cluster` so diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 60523c9cc..4cb455b2a 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,22 +5,16 @@ 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-storage-owner-rpc-domain-batch` +- Branch: `overtrue/arch-external-storage-api-boundary-phase` - 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/API-112/API-113/API-114/API-115/API-116/API-117/API-118/API-119/API-120/API-121/API-122/API-123/API-124/API-125/API-126/API-127/API-128/API-129/API-130/API-131/API-132/API-133/API-134/API-135/API-136/API-137/API-138/API-139/API-140/API-141/API-142/API-143/API-144/API-145/API-146/API-147/API-148/API-149/API-150/API-151/API-152/API-153/API-154/API-155/API-156/API-157/API-158/API-159/API-160/API-161/API-162/API-163/API-164/API-165/API-166/API-167/API-168/API-169/API-170/API-171/API-172/API-173/API-174/API-175/API-176/API-177/API-178/API-179/API-180/API-181/API-182/API-183/API-184/API-185/API-186/API-187/API-188/API-189/API-190/API-191/API-192/API-193/API-194/API-195/API-196/API-197/API-198/API-199/API-200/API-201/API-202/API-203/API-204/API-205/API-206/API-207/API-208/API-209/API-210/API-211/API-212/API-213/API-214/API-215/API-216/API-217/API-218/API-219/API-220/API-221/API-222/API-223/API-224/API-225/API-226/API-227/API-228/API-229/API-230/API-231/API-232/API-233/API-234/API-235/API-236/API-237/API-238/API-239/API-240/API-241/API-242/API-243/API-244/API-245/API-246/API-247/API-248/API-249/API-250/API-251/API-252/API-253/API-254/CTX-002`. -- Current phase PR: API-247 through API-255 storage owner/domain boundary - cleanup. -- Based on: rebased onto current `origin/main` after prerequisite PRs #3911, - #3913, and #3914 merged. +- Current baseline also includes API-255 from PR #3923. +- Current phase PR: API-256 external storage API boundary guard closure. +- Based on: current `origin/main` after PR #3923 merged. - PR type for this branch: `consumer-migration` -- Runtime behavior changes: none expected for API-255; root, app, and admin - facade modules consume the same storage owner symbols through - `crate::storage::storage_api` instead of direct storage root/module paths. -- Rust code changes: migrate `rustfs/src/storage_api.rs`, - `rustfs/src/app/storage_api.rs`, and `rustfs/src/admin/storage_api.rs` to - owner `storage_api` consumer modules; expose the needed access, - concurrency, deadlock, request-context, SSE, timeout, S3 API, and helper - consumer modules from the storage owner boundary; remove the storage root SSE - re-export. +- Runtime behavior changes: none expected for API-256; this is a + loss-prevention guard and documentation closure for the completed external + storage API boundary migration. +- Rust code changes: none expected. - CI/script changes: lock completed owner and test/fuzz boundaries against bare/glob imports, scattered raw ECStore facade subpaths, and startup runtime/root-server/table/S3/app shared/app bucket/app ECStore/admin facade @@ -47,8 +41,10 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block after API-252, reject restored parent wildcard imports anywhere under `rustfs/src/storage` after API-253, reject restoring storage owner root wildcard re-exports after API-254, reject direct storage owner paths from the - root/app/admin storage facades after API-255, and reject restoring storage - root SSE re-exports after API-255. + root/app/admin storage facades after API-255, reject restoring storage + root SSE re-exports after API-255, and reject direct external + `rustfs_ecstore::api` facade imports outside local `storage_api` boundary + files after API-256. ## Phase 0 Tasks @@ -5695,16 +5691,33 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block scan, migration guard, diff hygiene, and Rust risk scan passed; full PR gate is planned before PR. +- [x] `API-256` Close external storage API boundary guard coverage. + - Do: add one aggregate migration rule that rejects direct external + `rustfs_ecstore::api` facade imports outside local `storage_api` boundary + files after the runtime, test, and fuzz consumer migrations have been + narrowed. + - Acceptance: IAM, heal, scanner, notify, observability, Swift, S3 Select, + e2e, and fuzz source may only touch ECStore facade paths from their + dedicated storage API boundary files, with architecture docs recording the + phase closure rule. + - Must preserve: no Rust runtime behavior, public API, storage IO, + notification, IAM, scanner, heal, S3 Select, e2e, or fuzz behavior change. + - Verification: architecture migration guard, shell syntax check, diff + hygiene, full PR gate, and three-expert review passed. + ## Next PRs -1. `consumer-migration`: continue larger same-class storage owner cleanup - batches, prioritizing remaining root re-exports and consumer-domain guard - tightening. +1. `consumer-migration`: move to the next phase-level cleanup batch from the + current handoff, keeping behavior-owned ECStore internals in ECStore until a + pure-move slice is concrete. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| +| Quality/architecture | pass | API-256 closes external ECStore facade import coverage with one aggregate guard over runtime, test, e2e, and fuzz storage API boundaries. | +| Migration preservation | pass | The slice only adds guard/documentation coverage and does not move runtime symbols or alter external storage behavior. | +| Testing/verification | pass | Shell syntax, architecture migration guard, diff hygiene, script/docs risk review, and full PR gate passed. | | Quality/architecture | pass | API-255 moves the root, app, and admin storage facades onto the owner `storage_api` boundary and removes the remaining root SSE re-export. | | Migration preservation | pass | Root startup/server/protocol/workload/table paths, app bucket/object/multipart/runtime/SSE paths, and admin metrics/bucket/access facades keep the same underlying storage symbols. | | Testing/verification | pass | Focused RustFS compile, formatting, direct facade bypass scan, migration guard, diff hygiene, and diff-added Rust risk scan passed; full PR gate is planned before PR. | @@ -6011,6 +6024,16 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block Passed before push: +- Issue #660 API-256 current slice: + - Branch freshness check: based on current `origin/main` after PR #3923 + merged. + - `bash -n scripts/check_architecture_migration_rules.sh`: passed. + - `./scripts/check_architecture_migration_rules.sh`: passed. + - `git diff --check`: passed. + - External ECStore API aggregate boundary scan: passed. + - Script/docs risk scan: passed; no Rust source changed. + - `make pre-pr`: passed. + - Issue #660 API-255 current slice: - Branch freshness check: stacked on `overtrue/arch-storage-owner-root-export-cleanup` while prerequisite PRs diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index c62860168..bc90e6371 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -128,6 +128,7 @@ RUSTFS_STORAGE_API_ROOT_REEXPORT_HITS_FILE="${TMP_DIR}/rustfs_storage_api_root_r RUSTFS_APP_ADMIN_STORAGE_HELPER_ROOT_REEXPORT_HITS_FILE="${TMP_DIR}/rustfs_app_admin_storage_helper_root_reexport_hits.txt" EXTERNAL_TEST_ECSTORE_COMPAT_BYPASS_HITS_FILE="${TMP_DIR}/external_test_ecstore_compat_bypass_hits.txt" FUZZ_ECSTORE_COMPAT_BYPASS_HITS_FILE="${TMP_DIR}/fuzz_ecstore_compat_bypass_hits.txt" +EXTERNAL_ECSTORE_API_BOUNDARY_HITS_FILE="${TMP_DIR}/external_ecstore_api_boundary_hits.txt" ALL_STORAGE_COMPAT_SELF_FACADE_PATH_HITS_FILE="${TMP_DIR}/all_storage_compat_self_facade_path_hits.txt" RUSTFS_LOCAL_COMPAT_OWNER_SELF_PATH_HITS_FILE="${TMP_DIR}/rustfs_local_compat_owner_self_path_hits.txt" RUSTFS_ROOT_COMPAT_RELATIVE_CONSUMER_HITS_FILE="${TMP_DIR}/rustfs_root_compat_relative_consumer_hits.txt" @@ -2157,6 +2158,28 @@ if [[ -s "$FUZZ_ECSTORE_COMPAT_BYPASS_HITS_FILE" ]]; then report_failure "fuzz ECStore API imports must stay in fuzz storage_api boundary: $(paste -sd '; ' "$FUZZ_ECSTORE_COMPAT_BYPASS_HITS_FILE")" fi +( + cd "$ROOT_DIR" + rg -n --with-filename 'rustfs_ecstore::api::' \ + crates/e2e_test/src \ + crates/heal/src \ + crates/heal/tests \ + crates/iam/src \ + crates/notify/src \ + crates/obs/src \ + crates/protocols/src/swift \ + crates/s3select-api/src \ + crates/scanner/src \ + crates/scanner/tests \ + fuzz/fuzz_targets \ + --glob '*.rs' | + rg -v '^(crates/e2e_test/src/storage_api\.rs|crates/heal/src/heal/storage_api\.rs|crates/heal/tests/(endpoint_index_test|heal_bug_fixes_test|heal_integration_test)/storage_api\.rs|crates/iam/src/storage_api\.rs|crates/notify/src/storage_api\.rs|crates/obs/src/metrics/storage_api\.rs|crates/protocols/src/swift/storage_api\.rs|crates/s3select-api/src/storage_api\.rs|crates/scanner/src/storage_api\.rs|crates/scanner/tests/storage_api/mod\.rs|fuzz/fuzz_targets/(bucket_validation_storage_api|path_containment_storage_api)\.rs):' || true +) >"$EXTERNAL_ECSTORE_API_BOUNDARY_HITS_FILE" + +if [[ -s "$EXTERNAL_ECSTORE_API_BOUNDARY_HITS_FILE" ]]; then + report_failure "external ECStore API facade imports must stay in local storage_api boundary files: $(paste -sd '; ' "$EXTERNAL_ECSTORE_API_BOUNDARY_HITS_FILE")" +fi + ( cd "$ROOT_DIR" rg -n --with-filename '^(?:pub\(crate\) )?use rustfs_ecstore::api::[a-z_]+ as ecstore_[a-z_]+;' \