refactor(admin): resolve the object store through the injected server context slot (#4620)

backlog#1052 S2, second slice: admin request dispatch.

Admin operations are registered as &'static dyn Operation, so unlike FS
they cannot carry per-server state. Instead, the (per-server) S3Router now
holds the server's ServerContextSlot and injects it into every request's
extensions at both dispatch points (check_access and call) — the same
extensions channel already used for ReqInfo/RemoteAddr.

- make_admin_route binds the slot; the HTTP builder passes the same slot
  the S3 service (FS) uses, so both paths dispatch to the same server.
- admin/runtime_sources gains object_store_from_req / an extensions-based
  variant for handlers that have already moved request fields out; both
  fall back to the ambient process context when no slot was injected
  (direct handler tests, non-router paths) — single-instance unchanged.
- 27 handler resolution sites across 12 files now resolve per-request;
  helper functions without request access (11 sites: site_replication
  state helpers, quota/table_catalog internals, object_zip_download
  workers) stay on the ambient path until app subsystems become
  per-server (backlog#1052 S3).
- One site (site-replication resync) resolves before the request body is
  consumed, keeping the original error precedence.

New test: the router injects its slot into request extensions by pointer
identity. Embedded e2e (basic S3 + deferred-IAM recovery) still green.
This commit is contained in:
Zhengchao An
2026-07-09 19:42:52 +08:00
committed by GitHub
parent 071a4600bc
commit 90c0deff58
13 changed files with 115 additions and 43 deletions
+2 -2
View File
@@ -14,7 +14,7 @@
use crate::admin::auth::authenticate_request;
use crate::admin::router::{AdminOperation, Operation, S3Router};
use crate::admin::runtime_sources::{current_action_credentials, current_object_store_handle};
use crate::admin::runtime_sources::{current_action_credentials, object_store_from_req};
use crate::admin::storage_api::bucket::versioning_sys::BucketVersioningSys;
use crate::admin::storage_api::contract::admin::StorageAdminApi;
use crate::admin::storage_api::contract::bucket::{BucketOperations, BucketOptions};
@@ -114,7 +114,7 @@ async fn bucket_quota_limit(bucket: &str) -> Option<u64> {
#[async_trait::async_trait]
impl Operation for AccountInfoHandler {
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
+3 -3
View File
@@ -13,6 +13,7 @@
// limitations under the License.
use crate::admin::handlers::site_replication::site_replication_bucket_meta_hook;
use crate::admin::runtime_sources::object_store_from_extensions;
use crate::admin::storage_api::bucket::utils::{deserialize, serialize};
use crate::admin::storage_api::bucket::{
metadata::{
@@ -27,7 +28,6 @@ use crate::admin::storage_api::bucket::{
use crate::admin::storage_api::contract::bucket::{BucketOperations, BucketOptions, MakeBucketOptions};
use crate::admin::storage_api::error::StorageError;
use crate::{
admin::runtime_sources::current_object_store_handle,
admin::{
auth::validate_admin_request,
router::{AdminOperation, Operation, S3Router},
@@ -133,7 +133,7 @@ impl Operation for ExportBucketMetadata {
)
.await?;
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object store is not initialized"));
};
@@ -522,7 +522,7 @@ impl Operation for ImportBucketMetadata {
};
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object store is not initialized"));
};
+3 -3
View File
@@ -14,7 +14,7 @@
use crate::admin::auth::{authenticate_request, validate_admin_request};
use crate::admin::router::{AdminOperation, Operation, S3Router};
use crate::admin::runtime_sources::current_object_store_handle;
use crate::admin::runtime_sources::{object_store_from_extensions, object_store_from_req};
use crate::admin::storage_api::access::spawn_traced;
use crate::admin::storage_api::bucket::is_reserved_or_invalid_bucket;
use crate::admin::storage_api::bucket::utils::is_valid_object_prefix;
@@ -500,7 +500,7 @@ impl Operation for HealHandler {
// The heal channel currently models bucket/object work. Root heal reuses the
// existing format-heal path directly so `/v3/heal/` is accepted intentionally.
if should_handle_root_heal_directly(&hip) {
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
warn!(
event = EVENT_ADMIN_REQUEST_FAILED,
component = LOG_COMPONENT_ADMIN_API,
@@ -736,7 +736,7 @@ impl Operation for BackgroundHealStatusHandler {
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
validate_heal_admin_request(&req).await?;
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
warn!(
event = EVENT_ADMIN_REQUEST_FAILED,
component = LOG_COMPONENT_ADMIN_API,
+5 -5
View File
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use crate::admin::runtime_sources::object_store_from_extensions;
use http::{HeaderMap, HeaderValue, StatusCode, Uri};
use matchit::Params;
use rustfs_policy::policy::action::{Action, AdminAction};
@@ -26,8 +27,7 @@ use tracing::{error, info, warn};
use crate::{
admin::runtime_sources::{
AdminPoolStatus, QueryPoolStatusRequest, current_endpoints_handle, current_notification_system,
current_object_store_handle, default_admin_usecase,
AdminPoolStatus, QueryPoolStatusRequest, current_endpoints_handle, current_notification_system, default_admin_usecase,
},
admin::{
auth::validate_admin_request,
@@ -741,7 +741,7 @@ impl Operation for StartDecommission {
return Err(s3_error!(NotImplemented));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(decommission_admin_not_initialized_error_with_audit("start decommission", audit));
};
@@ -929,7 +929,7 @@ impl Operation for CancelDecommission {
.map_err(ApiError::from)
.map_err(|err| contextualize_admin_pool_api_error(err, "cancel decommission", format!("pool {idx}")))?;
} else {
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(decommission_admin_not_initialized_error_with_audit("cancel decommission", audit));
};
@@ -1042,7 +1042,7 @@ impl Operation for ClearDecommission {
.map_err(ApiError::from)
.map_err(|err| contextualize_admin_pool_api_error(err, "clear decommission", format!("pool {idx}")))?;
} else {
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(decommission_admin_not_initialized_error_with_audit("clear decommission", audit));
};
+5 -4
View File
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use crate::admin::runtime_sources::object_store_from_extensions;
use crate::admin::storage_api::contract::admin::StorageAdminApi;
use crate::admin::storage_api::contract::bucket::{BucketOperations, BucketOptions};
use crate::admin::storage_api::error::StorageError;
@@ -21,7 +22,7 @@ use crate::admin::storage_api::rebalance::{
};
use crate::admin::storage_api::runtime::{ECStore, NotificationSys};
use crate::{
admin::runtime_sources::{current_notification_system, current_object_store_handle},
admin::runtime_sources::current_notification_system,
admin::{
auth::validate_admin_request,
router::{AdminOperation, Operation, S3Router},
@@ -446,7 +447,7 @@ impl Operation for RebalanceStart {
return Err(s3_error!(InvalidArgument, "rebalance start does not accept query parameters"));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object layer is not initialized"));
};
@@ -630,7 +631,7 @@ impl Operation for RebalanceStatus {
)
.await?;
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object layer is not initialized"));
};
@@ -735,7 +736,7 @@ impl Operation for RebalanceStop {
return Err(s3_error!(InvalidArgument, "rebalance stop does not accept query parameters"));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object layer is not initialized"));
};
+7 -7
View File
@@ -15,7 +15,7 @@
use crate::admin::auth::validate_admin_request;
use crate::admin::handlers::site_replication::site_replication_peer_deployment_id_for_endpoint;
use crate::admin::router::{AdminOperation, Operation, S3Router};
use crate::admin::runtime_sources::{current_object_store_handle, current_replication_stats_handle, current_runtime_port};
use crate::admin::runtime_sources::{current_replication_stats_handle, current_runtime_port, object_store_from_req};
use crate::admin::storage_api::bucket::metadata::BUCKET_TARGETS_FILE;
use crate::admin::storage_api::bucket::metadata_sys;
use crate::admin::storage_api::bucket::metadata_sys::get_replication_config;
@@ -300,7 +300,7 @@ impl Operation for GetReplicationMetricsHandler {
return Err(s3_error!(InvalidRequest, "bucket is required"));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
@@ -358,7 +358,7 @@ impl Operation for SetRemoteTargetHandler {
return Err(s3_error!(InvalidRequest, "bucket is required"));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
@@ -506,7 +506,7 @@ impl Operation for ListRemoteTargetHandler {
return Err(s3_error!(InvalidRequest, "bucket is required"));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not initialized".to_string()));
};
@@ -567,7 +567,7 @@ impl Operation for RemoveRemoteTargetHandler {
return Err(s3_error!(InvalidRequest, "arn is required"));
};
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not initialized".to_string()));
};
@@ -673,7 +673,7 @@ impl Operation for ReplicationDiffHandler {
return Err(s3_error!(InvalidRequest, "bucket is required"));
};
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
@@ -835,7 +835,7 @@ impl Operation for ReplicationMrfHandler {
return Err(s3_error!(InvalidRequest, "bucket is required"));
};
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
@@ -18,6 +18,7 @@ use crate::admin::runtime_sources::{
current_deployment_id, current_endpoints_handle, current_iam_handle, current_object_store_handle, current_oidc_handle,
current_outbound_tls_generation, current_outbound_tls_state, current_region, current_replication_pool_handle,
current_replication_stats_handle, current_runtime_port, current_server_config, current_token_signing_key,
object_store_from_req,
};
use crate::admin::site_replication_identity::{
canonical_endpoint, deployment_id_for_endpoint, normalize_peer_map_by_identity_with, same_identity_endpoint,
@@ -5199,7 +5200,7 @@ impl Operation for SRPeerBucketOpsHandler {
.cloned()
.ok_or_else(|| s3_error!(InvalidRequest, "operation is required"))?;
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
@@ -5450,6 +5451,9 @@ impl Operation for SiteReplicationResyncOpHandler {
async fn call(&self, req: S3Request<Body>, _params: Params<'_, '_>) -> S3Result<S3Response<(StatusCode, Body)>> {
validate_site_replication_admin_request(&req, AdminAction::SiteReplicationResyncAction).await?;
let operation = query_pairs(&req.uri).get("operation").cloned().unwrap_or_default();
// Resolve before the request body is consumed below; the `else` stays
// at its original position so error precedence is unchanged.
let resolved_store = object_store_from_req(&req);
let peer: PeerInfo = read_site_replication_json(req, "", false).await?;
let _state_guard = SITE_REPLICATION_STATE_LOCK.lock().await;
let mut state = load_site_replication_state().await?;
@@ -5461,7 +5465,7 @@ impl Operation for SiteReplicationResyncOpHandler {
if !state.peers.contains_key(&peer.deployment_id) {
return Err(s3_error!(InvalidRequest, "site replication peer not found"));
}
let Some(store) = current_object_store_handle() else {
let Some(store) = resolved_store else {
return Err(S3Error::with_message(S3ErrorCode::InternalError, "Not init".to_string()));
};
let buckets = store.list_bucket(&BucketOptions::default()).await.map_err(ApiError::from)?;
+2 -3
View File
@@ -15,9 +15,8 @@
use super::{cluster_snapshot, metrics};
use crate::admin::auth::validate_admin_request;
use crate::admin::router::{AdminOperation, Operation, S3Router};
use crate::admin::runtime_sources::current_object_store_handle;
use crate::admin::runtime_sources::{
DefaultAdminUsecase, QueryServerInfoRequest, current_endpoints_handle, default_admin_usecase,
DefaultAdminUsecase, QueryServerInfoRequest, current_endpoints_handle, default_admin_usecase, object_store_from_req,
};
use crate::admin::storage_api::cluster::{
CapabilityState, CapabilityStatus, ObservabilitySnapshotProvider, TopologySnapshot, TopologySnapshotProvider,
@@ -527,7 +526,7 @@ impl Operation for InspectDataHandler {
));
};
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_req(&req) else {
log_system_request_failed!("inspect_data", "object_store_unavailable", "not initialized");
return Err(S3Error::with_message(S3ErrorCode::InternalError, "object store is not initialized"));
};
+5 -6
View File
@@ -13,15 +13,14 @@
// limitations under the License.
#![allow(unused_variables, unused_mut, unused_must_use)]
use crate::admin::runtime_sources::object_store_from_extensions;
use crate::admin::storage_api::tier::{
AdminError, DailyAllTierStats, ERR_TIER_ALREADY_EXISTS, ERR_TIER_BACKEND_IN_USE, ERR_TIER_BACKEND_NOT_EMPTY,
ERR_TIER_CONNECT_ERR, ERR_TIER_INVALID_CREDENTIALS, ERR_TIER_MISSING_CREDENTIALS, ERR_TIER_NAME_NOT_UPPERCASE,
ERR_TIER_NOT_FOUND, TierConfig, TierCreds, TierType, storageclass,
};
use crate::{
admin::runtime_sources::{
current_daily_tier_stats, current_notification_system, current_object_store_handle, current_tier_config_handle,
},
admin::runtime_sources::{current_daily_tier_stats, current_notification_system, current_tier_config_handle},
admin::{
auth::validate_admin_request,
router::{AdminOperation, Operation, S3Router},
@@ -328,7 +327,7 @@ impl Operation for AddTier {
&_ => (),
}
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object store is not initialized"));
};
@@ -475,7 +474,7 @@ impl Operation for EditTier {
"admin tier state"
);
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object store is not initialized"));
};
@@ -645,7 +644,7 @@ impl Operation for RemoveTier {
let tier_name = params.get("tiername").map(|s| s.to_string()).unwrap_or_default();
let Some(store) = current_object_store_handle() else {
let Some(store) = object_store_from_extensions(&req.extensions) else {
return Err(s3_error!(InternalError, "object store is not initialized"));
};
+5 -1
View File
@@ -48,8 +48,12 @@ use s3s::route::S3Route;
///
/// # Returns
/// An instance of S3Route for admin operations
pub fn make_admin_route(console_enabled: bool) -> std::io::Result<impl S3Route> {
pub fn make_admin_route(
console_enabled: bool,
server_ctx: std::sync::Arc<crate::admin::runtime_sources::ServerContextSlot>,
) -> std::io::Result<impl S3Route> {
let mut r: S3Router<AdminOperation> = S3Router::new(console_enabled);
r.set_server_ctx(server_ctx);
register_admin_routes(&mut r)?;
Ok(r)
}
+48 -1
View File
@@ -28,7 +28,7 @@ use super::storage_api::runtime::PeerRestClient;
use crate::admin::console::{is_console_path, make_console_server};
use crate::admin::handlers::oidc::is_oidc_path;
use crate::admin::runtime_sources::{
current_boot_time, current_bucket_monitor_handle, current_deployment_id, current_notification_system,
ServerContextSlot, current_boot_time, current_bucket_monitor_handle, current_deployment_id, current_notification_system,
current_object_store_handle, current_region, current_replication_pool_handle, current_replication_stats_handle,
current_server_config, default_object_usecase,
};
@@ -2295,6 +2295,10 @@ pub struct S3Router<T> {
router: Router<T>,
console_enabled: bool,
console_router: Option<axum::routing::RouterIntoService<Body>>,
/// This server's request-path context slot (backlog#1052 S2). Injected
/// into every request's extensions at dispatch so the static admin
/// operations can resolve their server's store.
server_ctx: Option<Arc<ServerContextSlot>>,
#[cfg(test)]
registered_routes: Vec<String>,
}
@@ -2338,11 +2342,18 @@ impl<T: Operation> S3Router<T> {
router,
console_enabled,
console_router,
server_ctx: None,
#[cfg(test)]
registered_routes: Vec::new(),
}
}
/// Bind this router to its server's context slot (backlog#1052 S2); the
/// slot is handed to every dispatched request via its extensions.
pub fn set_server_ctx(&mut self, server_ctx: Arc<ServerContextSlot>) {
self.server_ctx = Some(server_ctx);
}
pub fn insert(&mut self, method: Method, path: &str, operation: T) -> std::io::Result<()> {
let path = Self::make_route_str(method, path);
@@ -2430,6 +2441,9 @@ where
// check_access before call
async fn check_access(&self, req: &mut S3Request<Body>) -> S3Result<()> {
if let Some(server_ctx) = &self.server_ctx {
req.extensions.insert(server_ctx.clone());
}
if parse_replication_extension_request(&req.method, &req.uri).is_some()
|| parse_misc_extension_request(&req.method, &req.uri).is_some()
{
@@ -2495,6 +2509,9 @@ where
}
async fn call(&self, mut req: S3Request<Body>) -> S3Result<S3Response<Body>> {
if let Some(server_ctx) = &self.server_ctx {
req.extensions.insert(server_ctx.clone());
}
if let Some(ext_req) = parse_replication_extension_request(&req.method, &req.uri) {
return handle_replication_extension_request(&mut req, &ext_req).await;
}
@@ -3893,6 +3910,36 @@ mod tests {
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
}
// backlog#1052 S2: the router hands its server's context slot to every
// dispatched request via extensions, so the static admin operations can
// resolve their server's store instead of the process default.
#[tokio::test]
async fn check_access_injects_server_context_slot_into_request_extensions() {
let mut router: S3Router<AdminOperation> = S3Router::new(false);
let server_ctx = ServerContextSlot::new();
router.set_server_ctx(server_ctx.clone());
let mut req = S3Request {
input: Body::from(String::new()),
method: Method::GET,
uri: "/demo-bucket?replication-metrics".parse().expect("uri should parse"),
headers: HeaderMap::new(),
extensions: http::Extensions::new(),
credentials: None,
region: None,
service: None,
trailing_headers: None,
};
let _ = router.check_access(&mut req).await;
let injected = req
.extensions
.get::<Arc<ServerContextSlot>>()
.expect("dispatch must inject the server context slot");
assert!(Arc::ptr_eq(injected, &server_ctx), "the injected slot must be this router's slot");
}
#[tokio::test]
async fn check_access_rejects_anonymous_misc_extension_request() {
let router: S3Router<AdminOperation> = S3Router::new(false);
+22 -5
View File
@@ -21,11 +21,11 @@ pub(crate) use crate::app::admin_usecase::{
use crate::app::object_usecase::DefaultObjectUsecase;
use crate::runtime_sources as root_runtime_sources;
pub(crate) use crate::runtime_sources::{
AppContext, current_action_credentials, current_boot_time, current_bucket_metadata_handle, current_bucket_monitor_handle,
current_deployment_id, current_endpoints_handle, current_iam_handle, current_kms_runtime_service_manager,
current_notification_system_for_context, current_object_store_handle_for_context, current_oidc_handle,
current_ready_iam_handle, current_region, current_replication_pool_handle, current_replication_stats_handle,
current_server_config_for_context, current_token_signing_key,
AppContext, ServerContextSlot, current_action_credentials, current_boot_time, current_bucket_metadata_handle,
current_bucket_monitor_handle, current_deployment_id, current_endpoints_handle, current_iam_handle,
current_kms_runtime_service_manager, current_notification_system_for_context, current_object_store_handle_for_context,
current_oidc_handle, current_ready_iam_handle, current_region, current_replication_pool_handle,
current_replication_stats_handle, current_server_config_for_context, current_token_signing_key,
};
use rustfs_config::server_config::Config;
use rustfs_kms::KmsServiceManager;
@@ -52,6 +52,23 @@ pub(crate) fn current_object_store_handle() -> Option<Arc<ECStore>> {
current_object_store_handle_for_context(context.as_deref())
}
/// Resolve the object store for an admin request through the server's context
/// slot injected at router dispatch (backlog#1052 S2). Falls back to the
/// ambient process context when no slot was injected (direct handler tests,
/// paths outside the router) — the single-instance legacy default.
pub(crate) fn object_store_from_req<B>(req: &s3s::S3Request<B>) -> Option<Arc<ECStore>> {
object_store_from_extensions(&req.extensions)
}
/// Field-borrow form of [`object_store_from_req`] for handlers that have
/// already moved other request fields (body, credentials) out of the request.
pub(crate) fn object_store_from_extensions(extensions: &http::Extensions) -> Option<Arc<ECStore>> {
extensions
.get::<Arc<ServerContextSlot>>()
.and_then(|slot| slot.object_store())
.or_else(current_object_store_handle)
}
pub(crate) fn current_notification_system() -> Option<&'static NotificationSys> {
let context = current_app_context();
current_notification_system_for_context(context.as_deref())
+2 -1
View File
@@ -604,6 +604,7 @@ pub async fn start_http_server(
let s3_service = {
// Bind the S3 service to this server's context slot (backlog#1052 S2)
// so request dispatch resolves the server's own store.
let admin_server_ctx = server_ctx.clone();
let store = storage::ecfs::FS::with_server_ctx(server_ctx);
let mut b = S3ServiceBuilder::new(store.clone());
@@ -636,7 +637,7 @@ pub async fn start_http_server(
b.set_auth(IAMAuth::new(access_key, secret_key));
b.set_access(store);
b.set_route(storage::metadata_route::with_metadata_route(
admin::make_admin_route(config.console_enable)?,
admin::make_admin_route(config.console_enable, admin_server_ctx)?,
metadata_route_host,
));