From 1c58ee0d7eb8e10b6e7d200261e574fa18bed66a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Sat, 20 Jun 2026 22:20:26 +0800 Subject: [PATCH] refactor: return embedded identity from startup (#3666) --- docs/architecture/migration-progress.md | 63 +++++++++++++++++----- rustfs/src/embedded.rs | 24 ++++----- rustfs/src/startup_embedded.rs | 71 ++++++++++++++++++++++--- 3 files changed, 124 insertions(+), 34 deletions(-) diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 113a8c503..f28acc39c 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,17 +5,15 @@ 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-embedded-builder-shell` -- Baseline: completed `R-042/R-043`. -- Stacked on: embedded handle boundary slice. +- Branch: `overtrue/arch-embedded-startup-identity-flow` +- Baseline: completed `R-044/R-045`. +- Stacked on: embedded builder shell slice. - PR type for this branch: `pure-move` - Runtime behavior changes: none. -- Rust code changes: keep the public embedded builder as a thin shell over - crate-only startup arguments and move embedded port probing behind - startup-server helpers. -- CI/script changes: make the unsafe allowance checker avoid a local - `pipefail` false positive after `rg -q` matches nearby `SAFETY:` comments. -- Docs changes: record the embedded builder shell slices. +- Rust code changes: hide embedded startup argument fields behind crate-only + setters and return public server identity from the startup result. +- CI/script changes: none. +- Docs changes: record the embedded startup identity slices. ## Phase 0 Tasks @@ -2367,6 +2365,29 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block migration/layer guards, formatting, diff hygiene, Rust risk scan, branch freshness check, pre-commit quality gate, and three-expert review. +- [x] `R-046` Encapsulate embedded startup argument mutation. + - Do: hide embedded startup argument fields behind crate-only setter methods + used by the public builder. + - Acceptance: public builder fluent methods still apply the same address, + credential, region, and volume values in the same order. + - Must preserve: builder method signatures, default values, `volume` append + semantics, `volumes` replacement semantics, and startup input ownership. + - Verification: focused startup-embedded and embedded checks, RustFS lib + check, migration/layer guards, formatting, diff hygiene, Rust risk scan, + branch freshness check, pre-commit quality gate, and three-expert review. + +- [x] `R-047` Return embedded server identity from startup result. + - Do: let the crate-only startup result carry the access key, secret key, and + region used by the public server handle. + - Acceptance: public server accessors still expose the configured values + without the public builder duplicating identity assembly. + - Must preserve: startup error mapping, readiness logging order, endpoint + address handling, shutdown handle ownership, and no public API signature + changes. + - Verification: focused startup-embedded and embedded checks, RustFS lib + check, migration/layer guards, formatting, diff hygiene, Rust risk scan, + branch freshness check, pre-commit quality gate, and three-expert review. + - [x] `E-001/E-SET-001` Add ECStore layout skeleton and set-layout boundary. - Do: create the ECStore internal layout ownership buckets and pin static set layout versus runtime `Sets`/`SetDisks` orchestration boundaries before any @@ -2609,21 +2630,35 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Next PRs -1. `pure-move`: continue pruning residual embedded builder/startup-only - boundaries after the embedded builder shell slice lands. +1. `pure-move`: continue pruning residual embedded handle/startup-only + boundaries after the embedded startup identity slice lands. ## Pre-Push Review Log | Expert | Status | Notes | |---|---|---| -| Quality/architecture | passed | R-044 and R-045 keep the public embedded builder as a thin shell over crate-only startup args and move port probing behind startup-server helpers without widening the public API. | -| Migration preservation | passed | Builder defaults, fluent setters, volume replacement, credential/region accessors, public port helper signature, and localhost ephemeral port behavior stay unchanged. | -| Testing/verification | passed | Focused embedded/startup-embedded/startup-server checks, RustFS lib check, architecture/layer/unsafe guards, formatting, diff hygiene, Rust risk scan, and pre-commit gate passed. | +| Quality/architecture | passed | R-046 and R-047 hide startup argument mutation behind crate-only methods and move public server identity assembly into the startup result without widening the public API. | +| Migration preservation | passed | Builder setter semantics, public server identity accessors, endpoint handling, startup error mapping, and shutdown ownership stay unchanged. | +| Testing/verification | passed | Focused startup-embedded/embedded checks, RustFS lib check, architecture/layer/unsafe guards, formatting, diff hygiene, Rust risk scan, and pre-commit gate passed. | ## Verification Notes Passed before push: +- Issue #660 R-046/R-047 current slice: + - `cargo test -p rustfs --lib startup_embedded -- --nocapture`: passed. + - `cargo test -p rustfs --lib embedded -- --nocapture`: passed. + - `cargo check -p rustfs --lib`: passed. + - `cargo fmt --all --check`: passed. + - `git diff --check`: passed. + - `./scripts/check_architecture_migration_rules.sh`: passed. + - `./scripts/check_layer_dependencies.sh`: passed. + - `./scripts/check_unsafe_code_allowances.sh`: passed. + - Rust risk scan on changed Rust files: passed; matches were limited to + existing embedded doc examples. + - `make pre-commit`: passed; nextest ran 6324 tests with 6324 passed and + 111 skipped, and doctests passed. + - Issue #660 R-044/R-045 current slice: - `cargo test -p rustfs --lib embedded -- --nocapture`: passed. - `cargo test -p rustfs --lib startup_embedded -- --nocapture`: passed. diff --git a/rustfs/src/embedded.rs b/rustfs/src/embedded.rs index dff02cccc..952261963 100644 --- a/rustfs/src/embedded.rs +++ b/rustfs/src/embedded.rs @@ -160,25 +160,25 @@ impl RustFSServerBuilder { /// [`build`](Self::build), but that is too late for the earlier /// initialization that depends on the configured address. pub fn address(mut self, addr: impl Into) -> Self { - self.startup_args.address = addr.into(); + self.startup_args.set_address(addr.into()); self } /// Set the S3 access key (default: `"rustfsadmin"`). pub fn access_key(mut self, key: impl Into) -> Self { - self.startup_args.access_key = key.into(); + self.startup_args.set_access_key(key.into()); self } /// Set the S3 secret key (default: `"rustfsadmin"`). pub fn secret_key(mut self, key: impl Into) -> Self { - self.startup_args.secret_key = key.into(); + self.startup_args.set_secret_key(key.into()); self } /// Set the AWS region (default: `"us-east-1"`). pub fn region(mut self, region: impl Into) -> Self { - self.startup_args.region = region.into(); + self.startup_args.set_region(region.into()); self } @@ -187,13 +187,13 @@ impl RustFSServerBuilder { /// If no volumes are added, a temporary directory with a single drive is /// created automatically (and cleaned up on [`RustFSServer::shutdown`]). pub fn volume(mut self, path: impl Into) -> Self { - self.startup_args.volumes.push(path.into()); + self.startup_args.push_volume(path.into()); self } /// Set multiple volume paths at once, replacing any previously set volumes. pub fn volumes(mut self, paths: Vec) -> Self { - self.startup_args.volumes = paths; + self.startup_args.set_volumes(paths); self } @@ -212,17 +212,13 @@ impl RustFSServerBuilder { } async fn do_build(&mut self) -> Result { - let startup_args = self.startup_args.clone(); - let access_key = startup_args.access_key.clone(); - let secret_key = startup_args.secret_key.clone(); - let region = startup_args.region.clone(); - let started = run_embedded_startup(startup_args).await?; + let started = run_embedded_startup(self.startup_args.clone()).await?; let server = RustFSServer { address: started.bound_addr, - access_key, - secret_key, - region, + access_key: started.access_key, + secret_key: started.secret_key, + region: started.region, shutdown_handle: Some(started.shutdown_handle), cancel_token: started.cancel_token, temp_dir: started.temp_dir, diff --git a/rustfs/src/startup_embedded.rs b/rustfs/src/startup_embedded.rs index 7bba978c1..148122da1 100644 --- a/rustfs/src/startup_embedded.rs +++ b/rustfs/src/startup_embedded.rs @@ -28,11 +28,11 @@ use tokio_util::sync::CancellationToken; #[derive(Clone)] pub(crate) struct EmbeddedStartupArgs { - pub address: String, - pub access_key: String, - pub secret_key: String, - pub volumes: Vec, - pub region: String, + address: String, + access_key: String, + secret_key: String, + volumes: Vec, + region: String, } impl EmbeddedStartupArgs { @@ -45,10 +45,37 @@ impl EmbeddedStartupArgs { region: rustfs_config::RUSTFS_REGION.to_string(), } } + + pub(crate) fn set_address(&mut self, address: String) { + self.address = address; + } + + pub(crate) fn set_access_key(&mut self, access_key: String) { + self.access_key = access_key; + } + + pub(crate) fn set_secret_key(&mut self, secret_key: String) { + self.secret_key = secret_key; + } + + pub(crate) fn set_region(&mut self, region: String) { + self.region = region; + } + + pub(crate) fn push_volume(&mut self, path: String) { + self.volumes.push(path); + } + + pub(crate) fn set_volumes(&mut self, paths: Vec) { + self.volumes = paths; + } } pub(crate) struct EmbeddedStartedServer { pub bound_addr: SocketAddr, + pub access_key: String, + pub secret_key: String, + pub region: String, pub shutdown_handle: ShutdownHandle, pub cancel_token: CancellationToken, pub temp_dir: Option, @@ -68,13 +95,24 @@ impl From for EmbeddedStartupError { } pub(crate) async fn run_embedded_startup(args: EmbeddedStartupArgs) -> Result { + let EmbeddedStartupArgs { + address, + access_key, + secret_key, + volumes, + region, + } = args; + let server_access_key = access_key.clone(); + let server_secret_key = secret_key.clone(); + let server_region = region.clone(); + // Build is allowed to fail before irreversible global initialization // (for example on temporary I/O or directory setup errors), and in that // case callers can retry. let mut startup_guard = EmbeddedStartupGuard::new(); let EmbeddedStartupConfig { config, temp_dir_guard } = - prepare_embedded_startup_config(args.address, args.access_key, args.secret_key, args.volumes, args.region) + prepare_embedded_startup_config(address, access_key, secret_key, volumes, region) .await .map_err(init_error)?; @@ -134,6 +172,9 @@ pub(crate) async fn run_embedded_startup(args: EmbeddedStartupArgs) -> Result