gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-07-26 16:28:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: publish oidc through app context (#3893)

Browse Source
This commit is contained in:
Zhengchao An
2026-06-26 10:17:30 +08:00
committed by GitHub
parent c9614eb7cb
commit 1d5bf2699a
11 changed files with 117 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/architecture/compat-cleanup-register.md
-4
View File
@@ -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
docs/architecture/migration-progress.md
+10 -4
View File
@@ -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. |
rustfs/src/admin/console.rs
-1
View File
@@ -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()
rustfs/src/admin/handlers/oidc.rs
-5
View File
@@ -268,7 +268,6 @@ pub struct ListOidcProvidersHandler {}
#[async_trait::async_trait]
impl Operation for ListOidcProvidersHandler {
async fn call(&self, _req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
// 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<Body>, provider_id: &str) -> S3Result<String> {
// 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)
{
rustfs/src/admin/handlers/sts.rs
-2
View File
@@ -59,7 +59,6 @@ fn has_identity_authorization_context(policies: &[String], groups: &[String]) ->
}
fn configured_roles_claim_key(provider_id: &str) -> Option<String> {
// 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
rustfs/src/app/context.rs
+41 -21
View File
@@ -87,7 +87,12 @@ pub fn resolve_iam_handle() -> Option<Arc<IamSys<ObjectStore>>> {
/// Resolve OIDC system handle using AppContext-first precedence.
pub fn resolve_oidc_handle() -> Option<Arc<OidcSys>> {
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<OidcSys>) -> 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<Arc<AppContext>>,
fallback: impl FnOnce() -> Option<Arc<OidcSys>>,
) -> Option<Arc<OidcSys>> {
context.and_then(|context| context.oidc().handle()).or_else(fallback)
fn resolve_oidc_handle_with(context: Option<Arc<AppContext>>) -> Option<Arc<OidcSys>> {
context.and_then(|context| context.oidc().handle())
}
fn publish_oidc_handle_with(context: Option<Arc<AppContext>>, oidc: Arc<OidcSys>) -> 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<Arc<OidcSys>>,
token_signing_key: Option<String>,
}
@@ -606,22 +614,34 @@ mod tests {
self.ready
}
fn oidc(&self) -> Option<Arc<OidcSys>> {
self.oidc.clone()
}
fn token_signing_key(&self) -> Option<String> {
self.token_signing_key.clone()
}
}
struct TestOidcInterface {
oidc: Option<Arc<OidcSys>>,
oidc: StdRwLock<Option<Arc<OidcSys>>>,
}
impl TestOidcInterface {
fn new(oidc: Option<Arc<OidcSys>>) -> Self {
Self {
oidc: StdRwLock::new(oidc),
}
}
}
impl OidcInterface for TestOidcInterface {
fn handle(&self) -> Option<Arc<rustfs_iam::oidc::OidcSys>> {
self.oidc.clone()
self.oidc.read().ok().and_then(|oidc| oidc.as_ref().cloned())
}
fn publish_handle(&self, oidc: Arc<rustfs_iam::oidc::OidcSys>) -> 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
rustfs/src/app/context/global.rs
+9 -3
View File
@@ -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<IamSys<ObjectStore>>,
kms: Arc<KmsServiceManager>,
) -> 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<ECStore> {
@@ -131,6 +133,10 @@ impl AppContext {
self.oidc.clone()
}
pub fn publish_oidc_handle(&self, oidc: Arc<OidcSys>) -> bool {
self.oidc.publish_handle(oidc)
}
#[allow(dead_code)]
pub fn kms(&self) -> Arc<dyn KmsInterface> {
self.kms.clone()
rustfs/src/app/context/handles.rs
+36 -9
View File
@@ -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<Arc<OidcSys>> {
runtime_sources::oidc_handle()
}
fn token_signing_key(&self) -> Option<String> {
runtime_sources::token_signing_key()
}
}
/// Default OIDC interface adapter.
#[derive(Default)]
pub struct OidcHandle;
pub struct OidcHandle {
oidc: StdRwLock<Option<Arc<OidcSys>>>,
}
impl OidcHandle {
pub fn new(oidc: Option<Arc<OidcSys>>) -> Self {
Self {
oidc: StdRwLock::new(oidc),
}
}
}
impl Default for OidcHandle {
fn default() -> Self {
Self::new(None)
}
}
impl OidcInterface for OidcHandle {
fn handle(&self) -> Option<Arc<OidcSys>> {
runtime_sources::oidc_handle()
self.oidc.read().ok().and_then(|oidc| oidc.as_ref().cloned())
}
fn publish_handle(&self, oidc: Arc<OidcSys>) -> 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<dyn ActionCredentialInterfac
}
pub fn default_oidc_interface() -> Arc<dyn OidcInterface> {
Arc::new(OidcHandle)
Arc::new(OidcHandle::default())
}
pub fn oidc_interface(oidc: Option<Arc<OidcSys>>) -> Arc<dyn OidcInterface> {
Arc::new(OidcHandle::new(oidc))
}
pub fn default_region_interface() -> Arc<dyn RegionInterface> {
rustfs/src/app/context/interfaces.rs
+3 -3
View File
@@ -39,9 +39,6 @@ pub trait IamInterface: Send + Sync {
#[allow(dead_code)]
fn handle(&self) -> Arc<IamSys<ObjectStore>>;
fn is_ready(&self) -> bool;
fn oidc(&self) -> Option<Arc<OidcSys>> {
None
}
fn token_signing_key(&self) -> Option<String> {
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<Arc<OidcSys>>;
fn publish_handle(&self, _oidc: Arc<OidcSys>) -> bool {
false
}
}
/// KMS interface for application-layer use-cases.
rustfs/src/runtime_sources.rs
+2 -2
View File
@@ -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,
};
rustfs/src/startup_auth.rs
+16 -9
View File
@@ -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(())