From cbf526df8758c517204d3af734c908680f37be55 Mon Sep 17 00:00:00 2001 From: houseme Date: Wed, 24 Jun 2026 12:51:53 +0800 Subject: [PATCH] refactor(admin): migrate OIDC consumers to app context (#3800) --- docs/architecture/compat-cleanup-register.md | 4 ++- rustfs/src/admin/console.rs | 1 + rustfs/src/admin/handlers/oidc.rs | 5 +++ rustfs/src/admin/handlers/sts.rs | 2 ++ rustfs/src/admin/service/config.rs | 29 ++++++++++----- rustfs/src/admin/service/site_replication.rs | 11 ++++-- rustfs/src/app/context.rs | 12 ++++--- rustfs/src/server/module_switch.rs | 3 +- rustfs/src/server/readiness.rs | 6 ++-- rustfs/src/storage/rpc/bucket.rs | 2 +- rustfs/src/storage/rpc/health.rs | 2 +- rustfs/src/storage/rpc/node_service.rs | 37 +++++++++++++++----- scripts/layer-dependency-baseline.txt | 5 +++ 13 files changed, 88 insertions(+), 31 deletions(-) diff --git a/docs/architecture/compat-cleanup-register.md b/docs/architecture/compat-cleanup-register.md index 26d8e51d0..6b9d66301 100644 --- a/docs/architecture/compat-cleanup-register.md +++ b/docs/architecture/compat-cleanup-register.md @@ -12,7 +12,9 @@ for later deletion. ## Open Items -No compatibility code is currently registered. +- `CTX-002` + - Why: admin OIDC and STS consumers now resolve through `resolve_oidc_handle()`, but that resolver still falls back to legacy global OIDC state while the AppContext-owned OIDC wiring is being completed. + - Remove after: OIDC ownership is initialized and consumed through AppContext end-to-end, and `resolve_oidc_handle()` no longer needs the legacy global fallback path. ## Review Checklist diff --git a/rustfs/src/admin/console.rs b/rustfs/src/admin/console.rs index 0e9a0aaec..8ec26d1c9 100644 --- a/rustfs/src/admin/console.rs +++ b/rustfs/src/admin/console.rs @@ -136,6 +136,7 @@ impl Config { let http_prefix = rustfs_config::RUSTFS_HTTP_PREFIX; // Collect OIDC provider info if available + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. let oidc = resolve_oidc_handle() .map(|sys| { sys.list_visible_providers() diff --git a/rustfs/src/admin/handlers/oidc.rs b/rustfs/src/admin/handlers/oidc.rs index d1b0acdcb..d3419f290 100644 --- a/rustfs/src/admin/handlers/oidc.rs +++ b/rustfs/src/admin/handlers/oidc.rs @@ -268,6 +268,7 @@ pub struct ListOidcProvidersHandler {} #[async_trait::async_trait] impl Operation for ListOidcProvidersHandler { async fn call(&self, _req: S3Request, _params: Params<'_, '_>) -> S3Result> { + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. let oidc_sys = resolve_oidc_handle().ok_or_else(|| s3_error!(InternalError, "OIDC not initialized"))?; let providers = oidc_sys.list_visible_providers(); @@ -439,6 +440,7 @@ impl Operation for OidcAuthorizeHandler { return Err(s3_error!(InvalidRequest, "invalid provider_id")); } + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. let oidc_sys = resolve_oidc_handle().ok_or_else(|| s3_error!(InternalError, "OIDC not initialized"))?; // Derive the callback redirect URI from the request @@ -512,6 +514,7 @@ impl Operation for OidcCallbackHandler { )); } + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. let oidc_sys = resolve_oidc_handle().ok_or_else(|| s3_error!(InternalError, "OIDC not initialized"))?; let redirect_uri = derive_callback_uri(&req, provider_id)?; @@ -599,6 +602,7 @@ impl Operation for OidcLogoutHandler { return redirect_response(&fallback_location); }; + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. let location = match resolve_oidc_handle() { Some(oidc_sys) => match oidc_sys.build_logout_url(&logout_token, &fallback_location).await { Ok(Some(url)) => url, @@ -628,6 +632,7 @@ impl Operation for OidcLogoutHandler { /// an explicit redirect_uri is recommended to prevent header manipulation. fn derive_callback_uri(req: &S3Request, provider_id: &str) -> S3Result { // Use explicitly configured redirect_uri if available + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. if let Some(oidc_sys) = resolve_oidc_handle() && let Some(config) = oidc_sys.get_provider_config(provider_id) { diff --git a/rustfs/src/admin/handlers/sts.rs b/rustfs/src/admin/handlers/sts.rs index 9b608b2b9..6ff1e0206 100644 --- a/rustfs/src/admin/handlers/sts.rs +++ b/rustfs/src/admin/handlers/sts.rs @@ -59,6 +59,7 @@ fn has_identity_authorization_context(policies: &[String], groups: &[String]) -> } fn configured_roles_claim_key(provider_id: &str) -> Option { + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. resolve_oidc_handle() .as_ref() .and_then(|oidc_sys| oidc_sys.get_provider_config(provider_id)) @@ -316,6 +317,7 @@ async fn handle_assume_role_with_web_identity(body: AssumeRoleRequest) -> S3Resu } // Verify the JWT and extract claims + // RUSTFS_COMPAT_TODO(CTX-002): admin OIDC consumers still depend on the resolver's global fallback while AppContext OIDC wiring is incomplete. Remove after OIDC ownership moves fully into AppContext and the global fallback is retired. let oidc_sys = resolve_oidc_handle().ok_or_else(|| s3_error!(InternalError, "OIDC not initialized"))?; let (claims, provider_id) = oidc_sys diff --git a/rustfs/src/admin/service/config.rs b/rustfs/src/admin/service/config.rs index 592b8edb3..c4e85b7d2 100644 --- a/rustfs/src/admin/service/config.rs +++ b/rustfs/src/admin/service/config.rs @@ -15,7 +15,8 @@ use super::super::storageclass; use super::super::{STORAGE_CLASS_SUB_SYS, read_admin_config_without_migrate}; use crate::app::context::{ - publish_server_config, publish_storage_class_config, resolve_notification_system, resolve_object_store_handle, + AppContext, get_global_app_context, publish_server_config, publish_storage_class_config, resolve_notification_system, + resolve_object_store_handle, resolve_object_store_handle_for_context, }; use rustfs_audit::reload_audit_config; use rustfs_config::audit::{AUDIT_MQTT_SUB_SYS, AUDIT_REDIS_DEFAULT_CHANNEL, AUDIT_WEBHOOK_SUB_SYS}; @@ -66,6 +67,10 @@ fn invalid_request(message: impl Into) -> S3Error { S3Error::with_message(S3ErrorCode::InvalidRequest, message.into()) } +fn resolve_runtime_config_store_for_context(context: Option<&AppContext>) -> S3Result> { + resolve_object_store_handle_for_context(context).ok_or_else(|| internal_error("storage layer not initialized")) +} + async fn apply_storage_class_runtime_config(config: &ServerConfig) -> S3Result<()> { let Some(store) = resolve_object_store_handle() else { return Err(internal_error("storage layer not initialized")); @@ -292,14 +297,12 @@ pub async fn apply_dynamic_config_for_subsystem(config: &ServerConfig, sub_syste Ok(true) } -pub async fn reload_dynamic_config_runtime_state(sub_system: &str) -> S3Result<()> { +pub async fn reload_dynamic_config_runtime_state_for_context(context: Option<&AppContext>, sub_system: &str) -> S3Result<()> { if !is_dynamic_config_subsystem(sub_system) { return Err(internal_error(format!("unsupported dynamic config subsystem: {sub_system}"))); } - let Some(store) = resolve_object_store_handle() else { - return Err(internal_error("storage layer not initialized")); - }; + let store = resolve_runtime_config_store_for_context(context)?; let config = read_admin_config_without_migrate(store).await.map_err(|err| { warn!("peer reload_dynamic_config: failed to load server config for {sub_system}: {err}"); @@ -312,10 +315,13 @@ pub async fn reload_dynamic_config_runtime_state(sub_system: &str) -> S3Result<( Ok(()) } -pub async fn reload_runtime_config_snapshot() -> S3Result<()> { - let Some(store) = resolve_object_store_handle() else { - return Err(internal_error("storage layer not initialized")); - }; +pub async fn reload_dynamic_config_runtime_state(sub_system: &str) -> S3Result<()> { + let context = get_global_app_context(); + reload_dynamic_config_runtime_state_for_context(context.as_deref(), sub_system).await +} + +pub async fn reload_runtime_config_snapshot_for_context(context: Option<&AppContext>) -> S3Result<()> { + let store = resolve_runtime_config_store_for_context(context)?; let config = read_admin_config_without_migrate(store).await.map_err(|err| { warn!("peer reload_runtime_config_snapshot: failed to load server config: {err}"); @@ -340,6 +346,11 @@ pub async fn reload_runtime_config_snapshot() -> S3Result<()> { Ok(()) } +pub async fn reload_runtime_config_snapshot() -> S3Result<()> { + let context = get_global_app_context(); + reload_runtime_config_snapshot_for_context(context.as_deref()).await +} + pub async fn signal_dynamic_config_reload(sub_system: &str) { if !is_dynamic_config_subsystem(sub_system) { return; diff --git a/rustfs/src/admin/service/site_replication.rs b/rustfs/src/admin/service/site_replication.rs index b9145d629..896179713 100644 --- a/rustfs/src/admin/service/site_replication.rs +++ b/rustfs/src/admin/service/site_replication.rs @@ -15,7 +15,7 @@ use super::super::Error as StorageError; use super::super::{read_admin_config, save_admin_config}; use crate::admin::site_replication_identity::{deployment_id_for_endpoint, normalize_peer_map_by_identity_with}; -use crate::app::context::resolve_object_store_handle; +use crate::app::context::{AppContext, get_global_app_context, resolve_object_store_handle_for_context}; use rustfs_madmin::PeerInfo; use s3s::{S3Error, S3ErrorCode, S3Result}; use serde_json::{Map, Value}; @@ -90,8 +90,8 @@ fn normalize_site_replication_state_json(data: &[u8]) -> Result>, /// /// RustFS does not currently keep a separate in-memory cache for this state, /// so "reload" means validating that the persisted JSON is readable. -pub async fn reload_site_replication_runtime_state() -> S3Result<()> { - let Some(store) = resolve_object_store_handle() else { +pub async fn reload_site_replication_runtime_state_for_context(context: Option<&AppContext>) -> S3Result<()> { + let Some(store) = resolve_object_store_handle_for_context(context) else { return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string())); }; @@ -116,6 +116,11 @@ pub async fn reload_site_replication_runtime_state() -> S3Result<()> { } } +pub async fn reload_site_replication_runtime_state() -> S3Result<()> { + let context = get_global_app_context(); + reload_site_replication_runtime_state_for_context(context.as_deref()).await +} + #[cfg(test)] mod tests { use super::*; diff --git a/rustfs/src/app/context.rs b/rustfs/src/app/context.rs index 4b6f1c30d..5948cc6a8 100644 --- a/rustfs/src/app/context.rs +++ b/rustfs/src/app/context.rs @@ -84,16 +84,16 @@ pub fn resolve_iam_handle() -> Option>> { resolve_iam_handle_with(get_global_app_context(), rustfs_iam::get_global_iam_sys) } -/// Resolve a ready IAM system handle using AppContext-first precedence. -pub fn resolve_ready_iam_handle() -> rustfs_iam::error::Result>> { - resolve_ready_iam_handle_with(get_global_app_context(), rustfs_iam::get) -} - /// Resolve OIDC system handle using AppContext-first precedence. pub fn resolve_oidc_handle() -> Option> { resolve_oidc_handle_with(get_global_app_context(), rustfs_iam::get_oidc) } +/// Resolve a ready IAM system handle using AppContext-first precedence. +pub fn resolve_ready_iam_handle() -> rustfs_iam::error::Result>> { + resolve_ready_iam_handle_with(get_global_app_context(), rustfs_iam::get) +} + /// Resolve token signing key using AppContext-first precedence. pub fn resolve_token_signing_key() -> Option { resolve_token_signing_key_with(get_global_app_context(), rustfs_iam::manager::get_token_signing_key) @@ -1254,6 +1254,8 @@ mod tests { assert_eq!(resolve_outbound_tls_generation_with(None, || TlsGeneration(99)), TlsGeneration(99)); assert!(!resolve_iam_ready_with(None, || false)); assert!(resolve_iam_handle_with(None, || None).is_none()); + // OIDC fallback path: both context absent and fallback return None. + assert!(resolve_oidc_handle_with(None, || None).is_none()); assert!(Arc::ptr_eq( &resolve_bucket_metadata_handle_with(None, || Some(bucket_metadata.clone())).expect("fallback bucket metadata"), &bucket_metadata diff --git a/rustfs/src/server/module_switch.rs b/rustfs/src/server/module_switch.rs index 5a4f1f923..45e3178b9 100644 --- a/rustfs/src/server/module_switch.rs +++ b/rustfs/src/server/module_switch.rs @@ -12,7 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::storage::{Error as StorageError, read_config, resolve_object_store_handle, save_config}; +use crate::app::context::resolve_object_store_handle; +use crate::storage::{Error as StorageError, read_config, save_config}; use serde::{Deserialize, Serialize}; use std::sync::atomic::{AtomicBool, Ordering}; diff --git a/rustfs/src/server/readiness.rs b/rustfs/src/server/readiness.rs index e9b7f8fff..581c0e456 100644 --- a/rustfs/src/server/readiness.rs +++ b/rustfs/src/server/readiness.rs @@ -12,10 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::app::context::{resolve_endpoints_handle, resolve_iam_ready, resolve_lock_clients_handle}; +use crate::app::context::{ + resolve_endpoints_handle, resolve_iam_ready, resolve_lock_clients_handle, resolve_object_store_handle, +}; use crate::server::{ServiceState, ServiceStateManager}; use crate::server::{has_path_prefix, is_table_catalog_path}; -use crate::storage::{Endpoint, EndpointServerPools, is_dist_erasure, resolve_object_store_handle}; +use crate::storage::{Endpoint, EndpointServerPools, is_dist_erasure}; #[cfg(test)] use crate::storage::{Endpoints, PoolEndpoints}; use bytes::Bytes; diff --git a/rustfs/src/storage/rpc/bucket.rs b/rustfs/src/storage/rpc/bucket.rs index 6083157ae..63ea2fb72 100644 --- a/rustfs/src/storage/rpc/bucket.rs +++ b/rustfs/src/storage/rpc/bucket.rs @@ -42,7 +42,7 @@ impl NodeService { })); } - let Some(store) = resolve_object_store_handle() else { + let Some(store) = self.resolve_object_store() else { return Ok(Response::new(LoadBucketMetadataResponse { success: false, error_info: Some("errServerNotInitialized".to_string()), diff --git a/rustfs/src/storage/rpc/health.rs b/rustfs/src/storage/rpc/health.rs index 1422d41cd..92582a57d 100644 --- a/rustfs/src/storage/rpc/health.rs +++ b/rustfs/src/storage/rpc/health.rs @@ -215,7 +215,7 @@ impl NodeService { &self, _request: Request, ) -> Result, Status> { - let Some(store) = resolve_object_store_handle() else { + let Some(store) = self.resolve_object_store() else { return Ok(Response::new(LocalStorageInfoResponse { success: false, storage_info: Bytes::new(), diff --git a/rustfs/src/storage/rpc/node_service.rs b/rustfs/src/storage/rpc/node_service.rs index 2e77eed6a..fb5cc8b89 100644 --- a/rustfs/src/storage/rpc/node_service.rs +++ b/rustfs/src/storage/rpc/node_service.rs @@ -23,7 +23,9 @@ 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, resolve_lock_client}; +use crate::app::context::{ + AppContext, get_global_app_context, resolve_iam_handle, resolve_lock_client, resolve_object_store_handle_for_context, +}; use bytes::Bytes; use futures::Stream; use futures_util::future::join_all; @@ -171,17 +173,36 @@ mod lock; #[path = "metrics.rs"] mod metrics; -#[derive(Debug)] pub struct NodeService { local_peer: LocalPeerS3Client, + context: Option>, +} + +impl std::fmt::Debug for NodeService { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("NodeService") + .field("local_peer", &self.local_peer) + .field("context_present", &self.context.is_some()) + .finish() + } } pub fn make_server() -> NodeService { + let context = get_global_app_context(); + make_server_for_context(context) +} + +pub fn make_server_for_context(context: Option>) -> NodeService { let local_peer = LocalPeerS3Client::new(None, None); - NodeService { local_peer } + NodeService { local_peer, context } } impl NodeService { + fn resolve_object_store(&self) -> Option> { + let context = self.context.clone().or_else(get_global_app_context); + resolve_object_store_handle_for_context(context.as_deref()) + } + async fn find_disk(&self, disk_path: &str) -> Option { find_local_disk_by_ref(disk_path).await } @@ -900,7 +921,7 @@ impl Node for NodeService { &self, _request: Request, ) -> Result, Status> { - let Some(_store) = resolve_object_store_handle() else { + let Some(_store) = self.resolve_object_store() else { return Ok(Response::new(ReloadSiteReplicationConfigResponse { success: false, error_info: Some("errServerNotInitialized".to_string()), @@ -989,7 +1010,7 @@ impl Node for NodeService { &self, _request: Request, ) -> Result, Status> { - let Some(store) = resolve_object_store_handle() else { + let Some(store) = self.resolve_object_store() else { return Ok(Response::new(ReloadPoolMetaResponse { success: false, error_info: Some("errServerNotInitialized".to_string()), @@ -1014,7 +1035,7 @@ impl Node for NodeService { } async fn stop_rebalance(&self, request: Request) -> Result, Status> { - let Some(store) = resolve_object_store_handle() else { + let Some(store) = self.resolve_object_store() else { return Ok(Response::new(StopRebalanceResponse { success: false, error_info: Some("errServerNotInitialized".to_string()), @@ -1035,7 +1056,7 @@ impl Node for NodeService { request: Request, ) -> Result, Status> { let LoadRebalanceMetaRequest { start_rebalance } = request.into_inner(); - let Some(store) = resolve_object_store_handle() else { + let Some(store) = self.resolve_object_store() else { log_load_rebalance_meta_rejected!("server_not_initialized", start_rebalance); return Ok(Response::new(LoadRebalanceMetaResponse { success: false, @@ -1174,7 +1195,7 @@ impl Node for NodeService { &self, _request: Request, ) -> Result, Status> { - let Some(store) = resolve_object_store_handle() else { + let Some(store) = self.resolve_object_store() else { return Ok(Response::new(LoadTransitionTierConfigResponse { success: false, error_info: Some("errServerNotInitialized".to_string()), diff --git a/scripts/layer-dependency-baseline.txt b/scripts/layer-dependency-baseline.txt index 64b8e81a7..6a6bb94c2 100644 --- a/scripts/layer-dependency-baseline.txt +++ b/scripts/layer-dependency-baseline.txt @@ -47,6 +47,7 @@ dep|rustfs/src/init.rs|infra->interface|crate::admin dep|rustfs/src/protocols/client.rs|infra->app|crate::app::context::resolve_action_credentials dep|rustfs/src/protocols/client.rs|infra->interface|crate::storage::ecfs::FS dep|rustfs/src/server/audit.rs|infra->app|crate::app::context::resolve_server_config +dep|rustfs/src/server/module_switch.rs|infra->app|crate::app::context::resolve_object_store_handle dep|rustfs/src/server/event.rs|infra->app|crate::app::context::resolve_notify_interface dep|rustfs/src/server/event.rs|infra->app|crate::app::context::resolve_server_config dep|rustfs/src/server/http.rs|infra->interface|crate::admin @@ -57,6 +58,7 @@ dep|rustfs/src/server/layer.rs|infra->interface|crate::admin::handlers::health:: dep|rustfs/src/server/readiness.rs|infra->app|crate::app::context::resolve_endpoints_handle dep|rustfs/src/server/readiness.rs|infra->app|crate::app::context::resolve_iam_ready dep|rustfs/src/server/readiness.rs|infra->app|crate::app::context::resolve_lock_clients_handle +dep|rustfs/src/server/readiness.rs|infra->app|crate::app::context::resolve_object_store_handle dep|rustfs/src/server/tls_material.rs|infra->app|crate::app::context::resolve_outbound_tls_generation dep|rustfs/src/startup_bucket_metadata.rs|infra->app|crate::app::context::resolve_replication_pool_handle dep|rustfs/src/startup_tls_material.rs|infra->app|crate::app::context::resolve_outbound_tls_generation @@ -75,8 +77,11 @@ dep|rustfs/src/storage/helper.rs|infra->app|crate::app::context::resolve_notify_ dep|rustfs/src/storage/rpc/disk.rs|infra->app|crate::app::context::resolve_internode_metrics dep|rustfs/src/storage/rpc/health.rs|infra->app|crate::app::context::resolve_local_node_name dep|rustfs/src/storage/rpc/http_service.rs|infra->app|crate::app::context::resolve_internode_metrics +dep|rustfs/src/storage/rpc/node_service.rs|infra->app|crate::app::context::AppContext +dep|rustfs/src/storage/rpc/node_service.rs|infra->app|crate::app::context::get_global_app_context dep|rustfs/src/storage/rpc/node_service.rs|infra->app|crate::app::context::resolve_iam_handle dep|rustfs/src/storage/rpc/node_service.rs|infra->app|crate::app::context::resolve_lock_client +dep|rustfs/src/storage/rpc/node_service.rs|infra->app|crate::app::context::resolve_object_store_handle_for_context dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::config::reload_dynamic_config_runtime_state dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::config::reload_runtime_config_snapshot dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::site_replication::reload_site_replication_runtime_state