diff --git a/docs/architecture/compat-cleanup-register.md b/docs/architecture/compat-cleanup-register.md index 6b9d66301..bd6eebf08 100644 --- a/docs/architecture/compat-cleanup-register.md +++ b/docs/architecture/compat-cleanup-register.md @@ -12,10 +12,6 @@ for later deletion. ## Open Items -- `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 Before completing a PR that adds wrappers, re-exports, fallbacks, legacy action diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 52c301493..effa21a92 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,11 +5,11 @@ 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-ecstore-storage-api-import-boundaries` -- 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`. -- Based on: API-225 branch; branch routes ECStore internal storage contract imports through the owner-local `storage_api_contracts` boundary. +- Branch: `overtrue/arch-oidc-app-context-ownership` +- 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`. +- Based on: API-229 branch; branch retires the CTX-002 OIDC resolver fallback by publishing the initialized OIDC handle into AppContext. - PR type for this branch: `consumer-migration` -- Runtime behavior changes: none. +- Runtime behavior changes: admin OIDC resolution is now AppContext-owned after startup publishes the initialized OIDC handle; endpoint behavior and non-fatal OIDC initialization semantics remain unchanged. - Rust code changes: route replication pool, outbound TLS generation, runtime region, KMS encryption service, runtime support handles, S3 Select DB, internode RPC metrics, IAM authorization/handler reads, notification @@ -1465,6 +1465,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block metadata, object store, endpoints, tier config, server config, and buffer config. - Acceptance: context wins when present and global fallback works when absent. + - Cleanup update: retire the admin OIDC resolver's legacy global fallback by + publishing the initialized OIDC handle into AppContext and removing the + CTX-002 compatibility source/register entries. - Verification: focused resolver compatibility test, formatting, compile checks, migration guards, diff hygiene, Rust risk scan, and full `make pre-commit`. @@ -5346,6 +5349,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| +| Quality/architecture | pass | CTX-002 cleanup makes OIDC resolution AppContext-owned and removes the unused IAM-to-OIDC shortcut instead of keeping a hidden global bypass. | +| Migration preservation | pass | Startup still treats OIDC initialization as non-fatal and publishes the initialized handle when available; admin OIDC, STS, and console consumers keep the same response behavior. | +| Testing/verification | pass | Focused app::context tests, formatting, compatibility marker scan, and Rust risk scan passed; full PR gate is planned before PR. | | Quality/architecture | pass | API-229 routes ECStore benchmark erasure facade imports through a bench-local storage_api boundary. | | Migration preservation | pass | Erasure SIMD, comparison, streaming decode, and single-block non-inline benchmarks keep the same erasure implementations and behavior. | | Testing/verification | pass | Focused ECStore bench compile coverage, migration guard, residual bench API scan, Rust risk scan, formatting, and diff hygiene passed; layer guard and full PR gate are planned before PR. | diff --git a/rustfs/src/admin/console.rs b/rustfs/src/admin/console.rs index ff2f6ecf1..124368dcd 100644 --- a/rustfs/src/admin/console.rs +++ b/rustfs/src/admin/console.rs @@ -136,7 +136,6 @@ 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 ec03e0ba7..a0fd28d9c 100644 --- a/rustfs/src/admin/handlers/oidc.rs +++ b/rustfs/src/admin/handlers/oidc.rs @@ -268,7 +268,6 @@ 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(); @@ -440,7 +439,6 @@ 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 @@ -514,7 +512,6 @@ 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)?; @@ -602,7 +599,6 @@ 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, @@ -632,7 +628,6 @@ 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 30e92633c..f8437629a 100644 --- a/rustfs/src/admin/handlers/sts.rs +++ b/rustfs/src/admin/handlers/sts.rs @@ -59,7 +59,6 @@ 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)) @@ -317,7 +316,6 @@ 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/app/context.rs b/rustfs/src/app/context.rs index 004868190..e4ab21c3c 100644 --- a/rustfs/src/app/context.rs +++ b/rustfs/src/app/context.rs @@ -87,7 +87,12 @@ pub fn resolve_iam_handle() -> Option>> { /// Resolve OIDC system handle using AppContext-first precedence. pub fn resolve_oidc_handle() -> Option> { - resolve_oidc_handle_with(get_global_app_context(), runtime_sources::oidc_handle) + resolve_oidc_handle_with(get_global_app_context()) +} + +/// Publish the initialized OIDC system handle into the global AppContext. +pub fn publish_oidc_handle(oidc: Arc) -> bool { + publish_oidc_handle_with(get_global_app_context(), oidc) } /// Resolve a ready IAM system handle using AppContext-first precedence. @@ -319,11 +324,12 @@ fn resolve_iam_handle_with( context.map(|context| context.iam().handle()).or_else(fallback) } -fn resolve_oidc_handle_with( - context: Option>, - fallback: impl FnOnce() -> Option>, -) -> Option> { - context.and_then(|context| context.oidc().handle()).or_else(fallback) +fn resolve_oidc_handle_with(context: Option>) -> Option> { + context.and_then(|context| context.oidc().handle()) +} + +fn publish_oidc_handle_with(context: Option>, oidc: Arc) -> bool { + context.is_some_and(|context| context.publish_oidc_handle(oidc)) } fn resolve_ready_iam_handle_with( @@ -586,14 +592,16 @@ mod tests { server::dbms::{DatabaseManagerSystem, QueryHandle}, }; use std::path::PathBuf; - use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::{ + RwLock as StdRwLock, + atomic::{AtomicUsize, Ordering}, + }; use std::time::{Duration, SystemTime}; use tempfile::TempDir; use tokio_util::sync::CancellationToken; struct TestIamInterface { ready: bool, - oidc: Option>, token_signing_key: Option, } @@ -606,22 +614,34 @@ mod tests { self.ready } - fn oidc(&self) -> Option> { - self.oidc.clone() - } - fn token_signing_key(&self) -> Option { self.token_signing_key.clone() } } struct TestOidcInterface { - oidc: Option>, + oidc: StdRwLock>>, + } + + impl TestOidcInterface { + fn new(oidc: Option>) -> Self { + Self { + oidc: StdRwLock::new(oidc), + } + } } impl OidcInterface for TestOidcInterface { fn handle(&self) -> Option> { - self.oidc.clone() + self.oidc.read().ok().and_then(|oidc| oidc.as_ref().cloned()) + } + + fn publish_handle(&self, oidc: Arc) -> bool { + let Ok(mut published_oidc) = self.oidc.write() else { + return false; + }; + *published_oidc = Some(oidc); + true } } @@ -1058,12 +1078,9 @@ mod tests { AppContextTestInterfaces { iam: Arc::new(TestIamInterface { ready: true, - oidc: None, token_signing_key: Some(context_token_signing_key.clone()), }), - oidc: Arc::new(TestOidcInterface { - oidc: Some(context_oidc.clone()), - }), + oidc: Arc::new(TestOidcInterface::new(Some(context_oidc.clone()))), kms: Arc::new(TestKmsInterface { kms: context_kms.clone(), }), @@ -1160,8 +1177,11 @@ mod tests { context_outbound_tls_state.generation ); assert!(resolve_iam_ready_with(Some(context.clone()), || false)); - let resolved_oidc = resolve_oidc_handle_with(Some(context.clone()), || Some(fallback_oidc.clone())); + let resolved_oidc = resolve_oidc_handle_with(Some(context.clone())); assert!(resolved_oidc.as_ref().is_some_and(|oidc| Arc::ptr_eq(oidc, &context_oidc))); + assert!(publish_oidc_handle_with(Some(context.clone()), fallback_oidc.clone())); + let resolved_oidc = resolve_oidc_handle_with(Some(context.clone())); + assert!(resolved_oidc.as_ref().is_some_and(|oidc| Arc::ptr_eq(oidc, &fallback_oidc))); assert_eq!( resolve_token_signing_key_with(Some(context.clone()), || Some(fallback_token_signing_key.clone())).as_deref(), Some(context_token_signing_key.as_str()) @@ -1288,8 +1308,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!(resolve_oidc_handle_with(None).is_none()); + assert!(!publish_oidc_handle_with(None, context_oidc)); 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/app/context/global.rs b/rustfs/src/app/context/global.rs index 12cd7b958..e3d0a3909 100644 --- a/rustfs/src/app/context/global.rs +++ b/rustfs/src/app/context/global.rs @@ -23,7 +23,7 @@ use super::handles::{ default_outbound_tls_runtime_interface, default_performance_metrics_interface, default_region_interface, default_replication_pool_interface, default_replication_stats_interface, default_runtime_port_interface, default_s3select_db_interface, default_scanner_metrics_interface, default_server_config_interface, - default_storage_class_interface, default_tier_config_interface, default_tier_stats_interface, + default_storage_class_interface, default_tier_config_interface, default_tier_stats_interface, oidc_interface, }; use super::interfaces::{ ActionCredentialInterface, BootTimeInterface, BucketMetadataInterface, BucketMonitorInterface, BufferConfigInterface, @@ -33,7 +33,7 @@ use super::interfaces::{ ReplicationPoolInterface, ReplicationStatsInterface, RuntimePortInterface, S3SelectDbInterface, ScannerMetricsInterface, ServerConfigInterface, StorageClassInterface, TierConfigInterface, TierStatsInterface, }; -use rustfs_iam::{store::object::ObjectStore, sys::IamSys}; +use rustfs_iam::{oidc::OidcSys, store::object::ObjectStore, sys::IamSys}; use rustfs_kms::KmsServiceManager; use std::sync::{Arc, OnceLock}; @@ -116,7 +116,9 @@ impl AppContext { iam: Arc>, kms: Arc, ) -> Self { - Self::new(object_store, Arc::new(IamHandle::new(iam)), Arc::new(KmsHandle::new(kms))) + let mut context = Self::new(object_store, Arc::new(IamHandle::new(iam)), Arc::new(KmsHandle::new(kms))); + context.oidc = oidc_interface(super::runtime_sources::oidc_handle()); + context } pub fn object_store(&self) -> Arc { @@ -131,6 +133,10 @@ impl AppContext { self.oidc.clone() } + pub fn publish_oidc_handle(&self, oidc: Arc) -> bool { + self.oidc.publish_handle(oidc) + } + #[allow(dead_code)] pub fn kms(&self) -> Arc { self.kms.clone() diff --git a/rustfs/src/app/context/handles.rs b/rustfs/src/app/context/handles.rs index fd444bfdf..8be06c376 100644 --- a/rustfs/src/app/context/handles.rs +++ b/rustfs/src/app/context/handles.rs @@ -40,7 +40,11 @@ use rustfs_s3select_api::{QueryResult, server::dbms::DatabaseManagerSystem}; use rustfs_targets::{EventName, arn::TargetID}; use rustfs_tls_runtime::{GlobalPublishedOutboundTlsState, TlsGeneration}; use s3s::dto::SelectObjectContentInput; -use std::{collections::HashMap, sync::Arc, time::SystemTime}; +use std::{ + collections::HashMap, + sync::{Arc, RwLock as StdRwLock}, + time::SystemTime, +}; use tokio::sync::RwLock; /// Default IAM interface adapter. @@ -64,22 +68,41 @@ impl IamInterface for IamHandle { runtime_sources::ready_iam_handle().is_ok() } - fn oidc(&self) -> Option> { - runtime_sources::oidc_handle() - } - fn token_signing_key(&self) -> Option { runtime_sources::token_signing_key() } } /// Default OIDC interface adapter. -#[derive(Default)] -pub struct OidcHandle; +pub struct OidcHandle { + oidc: StdRwLock>>, +} + +impl OidcHandle { + pub fn new(oidc: Option>) -> Self { + Self { + oidc: StdRwLock::new(oidc), + } + } +} + +impl Default for OidcHandle { + fn default() -> Self { + Self::new(None) + } +} impl OidcInterface for OidcHandle { fn handle(&self) -> Option> { - runtime_sources::oidc_handle() + self.oidc.read().ok().and_then(|oidc| oidc.as_ref().cloned()) + } + + fn publish_handle(&self, oidc: Arc) -> bool { + let Ok(mut published_oidc) = self.oidc.write() else { + return false; + }; + *published_oidc = Some(oidc); + true } } @@ -486,7 +509,11 @@ pub fn default_action_credential_interface() -> Arc Arc { - Arc::new(OidcHandle) + Arc::new(OidcHandle::default()) +} + +pub fn oidc_interface(oidc: Option>) -> Arc { + Arc::new(OidcHandle::new(oidc)) } pub fn default_region_interface() -> Arc { diff --git a/rustfs/src/app/context/interfaces.rs b/rustfs/src/app/context/interfaces.rs index 5dacccc52..722f5771a 100644 --- a/rustfs/src/app/context/interfaces.rs +++ b/rustfs/src/app/context/interfaces.rs @@ -39,9 +39,6 @@ pub trait IamInterface: Send + Sync { #[allow(dead_code)] fn handle(&self) -> Arc>; fn is_ready(&self) -> bool; - fn oidc(&self) -> Option> { - None - } fn token_signing_key(&self) -> Option { None } @@ -50,6 +47,9 @@ pub trait IamInterface: Send + Sync { /// OIDC interface for admin and runtime consumers. pub trait OidcInterface: Send + Sync { fn handle(&self) -> Option>; + fn publish_handle(&self, _oidc: Arc) -> bool { + false + } } /// KMS interface for application-layer use-cases. diff --git a/rustfs/src/runtime_sources.rs b/rustfs/src/runtime_sources.rs index 6b8115b9a..2ff34849d 100644 --- a/rustfs/src/runtime_sources.rs +++ b/rustfs/src/runtime_sources.rs @@ -15,6 +15,6 @@ use crate::app::context; pub(crate) use context::{ - AppContext, resolve_action_credentials, resolve_buffer_config, resolve_notify_interface, resolve_ready_iam_handle, - resolve_region, resolve_replication_pool_handle, + AppContext, publish_oidc_handle, resolve_action_credentials, resolve_buffer_config, resolve_notify_interface, + resolve_ready_iam_handle, resolve_region, resolve_replication_pool_handle, }; diff --git a/rustfs/src/startup_auth.rs b/rustfs/src/startup_auth.rs index eeb058736..e8a8202d1 100644 --- a/rustfs/src/startup_auth.rs +++ b/rustfs/src/startup_auth.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use rustfs_iam::init_oidc_sys; +use rustfs_iam::{get_oidc, init_oidc_sys}; use std::io::{Error, Result}; use tracing::{error, info, warn}; @@ -44,14 +44,21 @@ pub(crate) async fn init_auth_integrations() -> Result<()> { } } - if let Err(e) = init_oidc_sys().await { - warn!( - event = EVENT_OIDC_INITIALIZATION_FAILED, - component = LOG_COMPONENT_MAIN, - subsystem = LOG_SUBSYSTEM_AUTH, - error = %e, - "OIDC initialization failed; continuing without OIDC providers" - ); + match init_oidc_sys().await { + Ok(()) => { + if let Some(oidc) = get_oidc() { + crate::runtime_sources::publish_oidc_handle(oidc); + } + } + Err(e) => { + warn!( + event = EVENT_OIDC_INITIALIZATION_FAILED, + component = LOG_COMPONENT_MAIN, + subsystem = LOG_SUBSYSTEM_AUTH, + error = %e, + "OIDC initialization failed; continuing without OIDC providers" + ); + } } Ok(())