diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 1a182bd7e..0c36b37b9 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,13 +5,13 @@ 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-rpc-iam-context-resolver` -- 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`. -- Based on: `main` after API-157. +- Branch: `overtrue/arch-rpc-node-context-resolvers` +- 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`. +- Based on: `main` after API-158. - PR type for this branch: `consumer-migration` - Runtime behavior changes: none. -- Rust code changes: route RPC node IAM operation consumers through an - AppContext IAM handle resolver with legacy global fallback. +- Rust code changes: route RPC node lock-client and local-node-name consumers + through AppContext resolvers with legacy global fallback. - 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 @@ -19,7 +19,7 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ECStore compatibility bypasses, plus runtime crate, owner crate, test/fuzz, and storage owner thin bridge regressions, plus app context and notify event-bridge thin module regressions. -- Docs changes: record the 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 owner facade cleanup. +- Docs changes: record the 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 owner facade cleanup. ## Phase 0 Tasks @@ -4285,6 +4285,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block migration guard, formatting, diff hygiene, Rust risk scan, branch freshness check, pre-commit, and three-expert review. +- [x] `API-159` Route RPC node lock and identity reads through AppContext. + - Do: add lock-client and local-node-name AppContext interfaces, default + legacy adapters, resolver helpers, and use them in RPC node lock and health + handlers. + - Acceptance: RPC node lock operations and health metric node identity no + longer read legacy global state directly when AppContext owns the boundary. + - Must preserve: lock-client initialization error text, health metric node + labels, async local-node-name behavior, and legacy global fallback when + AppContext is absent. + - Verification: RustFS compile coverage, targeted context resolver tests, + migration guard, formatting, diff hygiene, Rust risk scan, branch freshness + check, pre-commit, and three-expert review. + ## Next PRs 1. `consumer-migration`: continue reducing direct global reads behind AppContext resolver boundaries. @@ -4314,11 +4327,29 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Quality/architecture | pass | API-158 keeps RPC node IAM operations behind the AppContext-owned IAM handle resolver boundary. | | Migration preservation | pass | RPC IAM policy, user, group, and service-account operations keep validation, arguments, and legacy fallback behavior. | | Testing/verification | pass | RustFS focused compile, targeted context tests, formatting, migration guard, diff hygiene, Rust risk scan, and pre-commit passed for API-158. | +| Quality/architecture | pass | API-159 keeps RPC node lock client and node identity reads behind AppContext resolver boundaries. | +| Migration preservation | pass | RPC lock initialization errors and health metric node-name inputs keep legacy fallback behavior. | +| Testing/verification | pass | RustFS focused compile, targeted context tests, formatting, migration guard, diff hygiene, Rust risk scan, and pre-commit passed for API-159. | ## Verification Notes Passed before push: +- Issue #660 API-159 current slice: + - `cargo check --tests -p rustfs`: passed. + - `cargo test -p rustfs resolver_helpers_are_context_first_and_fallback_when_context_is_absent --lib`: + 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. + - `make pre-commit`: passed. + - AppContext RPC node resolver scan: passed; direct RPC node lock-client and + local-node-name global reads are isolated to AppContext default adapters. + - Rust risk scan: no new production unwrap/expect, panic/todo/unsafe, or + cast risks added. + - Issue #660 API-158 current slice: - `cargo check --tests -p rustfs`: passed. - `cargo test -p rustfs resolver_helpers_are_context_first_and_fallback_when_context_is_absent --lib`: diff --git a/rustfs/src/app/context.rs b/rustfs/src/app/context.rs index d28435fc8..7e2dec10d 100644 --- a/rustfs/src/app/context.rs +++ b/rustfs/src/app/context.rs @@ -34,7 +34,8 @@ use crate::config::RustFSBufferConfig; use rustfs_config::server_config::Config; use rustfs_iam::{store::object::ObjectStore, sys::IamSys}; use rustfs_kms::KmsServiceManager; -use std::sync::Arc; +use rustfs_lock::LockClient; +use std::{future::Future, sync::Arc}; use tokio::sync::RwLock; /// Resolve KMS runtime service manager using AppContext-first precedence. @@ -82,6 +83,16 @@ pub fn resolve_endpoints_handle() -> Option { resolve_endpoints_handle_with(get_global_app_context(), || default_endpoints_interface().handle()) } +/// Resolve lock client using AppContext-first precedence. +pub fn resolve_lock_client() -> Option> { + resolve_lock_client_with(get_global_app_context(), || default_lock_client_interface().handle()) +} + +/// Resolve local node name using AppContext-first precedence. +pub async fn resolve_local_node_name() -> String { + resolve_local_node_name_with(get_global_app_context(), rustfs_common::get_global_local_node_name).await +} + /// Resolve tier config handle using AppContext-first precedence. pub fn resolve_tier_config_handle() -> Arc> { resolve_tier_config_handle_with(get_global_app_context(), || default_tier_config_interface().handle()) @@ -141,6 +152,25 @@ fn resolve_endpoints_handle_with( context.and_then(|context| context.endpoints().handle()).or_else(fallback) } +fn resolve_lock_client_with( + context: Option>, + fallback: impl FnOnce() -> Option>, +) -> Option> { + context.and_then(|context| context.lock_client().handle()).or_else(fallback) +} + +async fn resolve_local_node_name_with(context: Option>, fallback: F) -> String +where + F: FnOnce() -> Fut, + Fut: Future, +{ + if let Some(context) = context { + return context.local_node_name().get().await; + } + + fallback().await +} + fn resolve_tier_config_handle_with( context: Option>, fallback: impl FnOnce() -> Arc>, @@ -170,10 +200,12 @@ mod tests { use crate::app::context::handles::{default_notify_interface, default_region_interface}; use crate::app::context::interfaces::{ BucketMetadataInterface, BufferConfigInterface, EndpointsInterface, IamInterface, KmsInterface, KmsRuntimeInterface, - ServerConfigInterface, TierConfigInterface, + LocalNodeNameInterface, LockClientInterface, ServerConfigInterface, TierConfigInterface, }; use crate::config::{RustFSBufferConfig, WorkloadProfile}; + use async_trait::async_trait; use rustfs_iam::{store::object::ObjectStore, sys::IamSys}; + use rustfs_lock::{LocalClient, LockClient}; use std::path::PathBuf; use tempfile::TempDir; use tokio_util::sync::CancellationToken; @@ -232,6 +264,27 @@ mod tests { } } + struct TestLockClientInterface { + client: Option>, + } + + impl LockClientInterface for TestLockClientInterface { + fn handle(&self) -> Option> { + self.client.clone() + } + } + + struct TestLocalNodeNameInterface { + name: String, + } + + #[async_trait] + impl LocalNodeNameInterface for TestLocalNodeNameInterface { + async fn get(&self) -> String { + self.name.clone() + } + } + struct TestTierConfigInterface { tier_config: Arc>, } @@ -321,6 +374,10 @@ mod tests { let tier_config = TierConfigMgr::new(); let server_config = Config::new(); let buffer_config = RustFSBufferConfig::new(WorkloadProfile::AiTraining); + let context_lock_client: Arc = Arc::new(LocalClient::new()); + let fallback_lock_client: Arc = Arc::new(LocalClient::new()); + let context_node_name = "context-node".to_string(); + let fallback_node_name = "fallback-node".to_string(); let context = Arc::new(AppContext::with_test_interfaces( object_store.clone(), @@ -339,6 +396,12 @@ mod tests { endpoints: Arc::new(TestEndpointsInterface { endpoints: Some(endpoints.clone()), }), + lock_client: Arc::new(TestLockClientInterface { + client: Some(context_lock_client.clone()), + }), + local_node_name: Arc::new(TestLocalNodeNameInterface { + name: context_node_name.clone(), + }), region: default_region_interface(), tier_config: Arc::new(TestTierConfigInterface { tier_config: tier_config.clone(), @@ -371,6 +434,14 @@ mod tests { .drives_per_set, endpoints.as_ref()[0].drives_per_set ); + assert!(Arc::ptr_eq( + &resolve_lock_client_with(Some(context.clone()), || None).expect("context lock client"), + &context_lock_client + )); + assert_eq!( + resolve_local_node_name_with(Some(context.clone()), || async { fallback_node_name.clone() }).await, + context_node_name + ); assert!(Arc::ptr_eq( &resolve_tier_config_handle_with(Some(context.clone()), TierConfigMgr::new), &tier_config @@ -405,6 +476,14 @@ mod tests { .drives_per_set, endpoints.as_ref()[0].drives_per_set ); + assert!(Arc::ptr_eq( + &resolve_lock_client_with(None, || Some(fallback_lock_client.clone())).expect("fallback lock client"), + &fallback_lock_client + )); + assert_eq!( + resolve_local_node_name_with(None, || async { fallback_node_name.clone() }).await, + fallback_node_name + ); assert!(Arc::ptr_eq(&resolve_tier_config_handle_with(None, || tier_config.clone()), &tier_config)); assert_eq!( resolve_server_config_with(None, || Some(server_config.clone())).expect("fallback server config"), diff --git a/rustfs/src/app/context/global.rs b/rustfs/src/app/context/global.rs index 2ef96bcea..3dff8a3b2 100644 --- a/rustfs/src/app/context/global.rs +++ b/rustfs/src/app/context/global.rs @@ -15,12 +15,12 @@ use super::super::{ECStore, set_object_store_resolver}; use super::handles::{ IamHandle, KmsHandle, default_bucket_metadata_interface, default_buffer_config_interface, default_endpoints_interface, - default_kms_runtime_interface, default_notify_interface, default_region_interface, default_server_config_interface, - default_tier_config_interface, + default_kms_runtime_interface, default_local_node_name_interface, default_lock_client_interface, default_notify_interface, + default_region_interface, default_server_config_interface, default_tier_config_interface, }; use super::interfaces::{ BucketMetadataInterface, BufferConfigInterface, EndpointsInterface, IamInterface, KmsInterface, KmsRuntimeInterface, - NotifyInterface, RegionInterface, ServerConfigInterface, TierConfigInterface, + LocalNodeNameInterface, LockClientInterface, NotifyInterface, RegionInterface, ServerConfigInterface, TierConfigInterface, }; use rustfs_iam::{store::object::ObjectStore, sys::IamSys}; use rustfs_kms::KmsServiceManager; @@ -37,6 +37,8 @@ pub struct AppContext { notify: Arc, bucket_metadata: Arc, endpoints: Arc, + lock_client: Arc, + local_node_name: Arc, region: Arc, tier_config: Arc, server_config: Arc, @@ -53,6 +55,8 @@ impl AppContext { notify: default_notify_interface(), bucket_metadata: default_bucket_metadata_interface(), endpoints: default_endpoints_interface(), + lock_client: default_lock_client_interface(), + local_node_name: default_local_node_name_interface(), region: default_region_interface(), tier_config: default_tier_config_interface(), server_config: default_server_config_interface(), @@ -97,6 +101,14 @@ impl AppContext { self.endpoints.clone() } + pub fn lock_client(&self) -> Arc { + self.lock_client.clone() + } + + pub fn local_node_name(&self) -> Arc { + self.local_node_name.clone() + } + pub fn region(&self) -> Arc { self.region.clone() } @@ -122,6 +134,8 @@ pub(super) struct AppContextTestInterfaces { pub(super) notify: Arc, pub(super) bucket_metadata: Arc, pub(super) endpoints: Arc, + pub(super) lock_client: Arc, + pub(super) local_node_name: Arc, pub(super) region: Arc, pub(super) tier_config: Arc, pub(super) server_config: Arc, @@ -139,6 +153,8 @@ impl AppContext { notify: interfaces.notify, bucket_metadata: interfaces.bucket_metadata, endpoints: interfaces.endpoints, + lock_client: interfaces.lock_client, + local_node_name: interfaces.local_node_name, region: interfaces.region, tier_config: interfaces.tier_config, server_config: interfaces.server_config, diff --git a/rustfs/src/app/context/handles.rs b/rustfs/src/app/context/handles.rs index bb0e2ecbb..63bc03d95 100644 --- a/rustfs/src/app/context/handles.rs +++ b/rustfs/src/app/context/handles.rs @@ -15,17 +15,19 @@ use super::super::EndpointServerPools; use super::super::TierConfigMgr; use super::super::metadata_sys::{BucketMetadataSys, get_global_bucket_metadata_sys}; -use super::super::{get_global_endpoints_opt, get_global_region, get_global_tier_config_mgr}; +use super::super::{get_global_endpoints_opt, get_global_lock_client, get_global_region, get_global_tier_config_mgr}; use super::interfaces::{ BucketMetadataInterface, BufferConfigInterface, EndpointsInterface, IamInterface, KmsInterface, KmsRuntimeInterface, - NotifyInterface, RegionInterface, ServerConfigInterface, TierConfigInterface, + LocalNodeNameInterface, LockClientInterface, NotifyInterface, RegionInterface, ServerConfigInterface, TierConfigInterface, }; use crate::config::{RustFSBufferConfig, get_global_buffer_config}; use async_trait::async_trait; +use rustfs_common::get_global_local_node_name; use rustfs_config::server_config::Config; use rustfs_config::server_config::get_global_server_config; use rustfs_iam::{store::object::ObjectStore, sys::IamSys}; use rustfs_kms::{KmsServiceManager, get_global_kms_service_manager}; +use rustfs_lock::LockClient; use rustfs_notify::{EventArgs, NotificationError, notifier_global}; use rustfs_targets::{EventName, arn::TargetID}; use std::sync::Arc; @@ -125,6 +127,27 @@ impl EndpointsInterface for EndpointsHandle { } } +/// Default lock client interface adapter. +#[derive(Default)] +pub struct LockClientHandle; + +impl LockClientInterface for LockClientHandle { + fn handle(&self) -> Option> { + get_global_lock_client() + } +} + +/// Default local node name interface adapter. +#[derive(Default)] +pub struct LocalNodeNameHandle; + +#[async_trait] +impl LocalNodeNameInterface for LocalNodeNameHandle { + async fn get(&self) -> String { + get_global_local_node_name().await + } +} + /// Default region interface adapter. #[derive(Default)] pub struct RegionHandle; @@ -181,6 +204,14 @@ pub fn default_endpoints_interface() -> Arc { Arc::new(EndpointsHandle) } +pub fn default_lock_client_interface() -> Arc { + Arc::new(LockClientHandle) +} + +pub fn default_local_node_name_interface() -> Arc { + Arc::new(LocalNodeNameHandle) +} + pub fn default_region_interface() -> Arc { Arc::new(RegionHandle) } diff --git a/rustfs/src/app/context/interfaces.rs b/rustfs/src/app/context/interfaces.rs index 28066f7ec..7a9ef1d26 100644 --- a/rustfs/src/app/context/interfaces.rs +++ b/rustfs/src/app/context/interfaces.rs @@ -20,6 +20,7 @@ use async_trait::async_trait; use rustfs_config::server_config::Config; use rustfs_iam::{store::object::ObjectStore, sys::IamSys}; use rustfs_kms::KmsServiceManager; +use rustfs_lock::LockClient; use rustfs_notify::{EventArgs, NotificationError}; use rustfs_targets::{EventName, arn::TargetID}; use std::sync::Arc; @@ -68,6 +69,17 @@ pub trait EndpointsInterface: Send + Sync { fn handle(&self) -> Option; } +/// Lock client interface for application-layer use-cases. +pub trait LockClientInterface: Send + Sync { + fn handle(&self) -> Option>; +} + +/// Local node name interface for application-layer use-cases. +#[async_trait] +pub trait LocalNodeNameInterface: Send + Sync { + async fn get(&self) -> String; +} + /// Region interface for application-layer use-cases. pub trait RegionInterface: Send + Sync { fn get(&self) -> Option; diff --git a/rustfs/src/app/mod.rs b/rustfs/src/app/mod.rs index 1a0528926..11b70c154 100644 --- a/rustfs/src/app/mod.rs +++ b/rustfs/src/app/mod.rs @@ -630,6 +630,10 @@ pub(crate) fn get_global_endpoints_opt() -> Option { crate::storage::get_global_endpoints_opt() } +pub(crate) fn get_global_lock_client() -> Option> { + crate::storage::get_global_lock_client() +} + pub(crate) fn get_global_region() -> Option { crate::storage::get_global_region() } diff --git a/rustfs/src/storage/rpc/health.rs b/rustfs/src/storage/rpc/health.rs index 06354ada1..1422d41cd 100644 --- a/rustfs/src/storage/rpc/health.rs +++ b/rustfs/src/storage/rpc/health.rs @@ -13,6 +13,7 @@ // limitations under the License. use super::*; +use crate::app::context::resolve_local_node_name; use crate::storage::rpc::encode_msgpack_map; use rustfs_storage_api::StorageAdminApi; @@ -21,7 +22,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let addr = get_global_local_node_name().await; + let addr = resolve_local_node_name().await; let info = get_proc_info(&addr); match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(GetProcInfoResponse { @@ -41,7 +42,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let addr = get_global_local_node_name().await; + let addr = resolve_local_node_name().await; let info = get_mem_info(&addr); match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(GetMemInfoResponse { @@ -61,7 +62,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let addr = get_global_local_node_name().await; + let addr = resolve_local_node_name().await; let info = get_sys_errors(&addr); match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(GetSysErrorsResponse { @@ -81,7 +82,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let addr = get_global_local_node_name().await; + let addr = resolve_local_node_name().await; let info = get_sys_config(&addr); match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(GetSysConfigResponse { @@ -101,7 +102,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let addr = get_global_local_node_name().await; + let addr = resolve_local_node_name().await; let info = get_sys_services(&addr); match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(GetSeLinuxInfoResponse { @@ -159,7 +160,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let addr = get_global_local_node_name().await; + let addr = resolve_local_node_name().await; let info = get_net_info(&addr, ""); match encode_msgpack_map(&info) { Ok(buf) => Ok(Response::new(GetNetInfoResponse { diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index ab682c966..30bf7f1a2 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -16,19 +16,19 @@ use super::super::{ 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, + collect_local_metrics, find_local_disk_by_ref, 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 crate::app::context::resolve_iam_handle; +use crate::app::context::{resolve_iam_handle, resolve_lock_client}; use bytes::Bytes; use futures::Stream; use futures_util::future::join_all; use rmp_serde::Deserializer; -use rustfs_common::{get_global_local_node_name, heal_channel::HealOpts}; +use rustfs_common::heal_channel::HealOpts; use rustfs_filemeta::{FileInfo, MetacacheReader}; use rustfs_iam::store::UserType; use rustfs_lock::{LockClient, LockRequest}; @@ -172,9 +172,9 @@ impl NodeService { all_local_disk_path().await } - /// Get the global lock client, returning an error if not initialized + /// Get the lock client, returning an error if not initialized fn get_lock_client(&self) -> Result, Status> { - get_global_lock_client() + resolve_lock_client() .ok_or_else(|| Status::internal("Lock client not initialized. Please ensure storage is initialized first.")) } }