refactor: remove service resolver fallbacks (#3955)

This commit is contained in:
Zhengchao An
2026-06-27 19:35:43 +08:00
committed by GitHub
parent 4732f4ee2d
commit 243a20b14e
5 changed files with 140 additions and 73 deletions
+45 -11
View File
@@ -5,7 +5,7 @@ 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-runtime-fallback-boundary-cleanup`
- Branch: `overtrue/arch-runtime-default-resolver-fallback-removal`
- 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/API-230/API-231/API-232/API-233/API-234/API-235/API-236/API-237/API-238/API-239/API-240/API-241/API-242/API-243/API-244/API-245/API-246/API-247/API-248/API-249/API-250/API-251/API-252/API-253/API-254/CTX-002`.
- Current baseline also includes API-255 from PR #3923, API-256 from PR
#3925, CFG-009 from PR #3927, C-007/C-009 from PR #3935, C-008/C-010
@@ -21,18 +21,24 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
alias sweep from PR #3949, the GLOB-007 object-store fallback removal from
PR #3950, the GLOB-007 notification-system/server-config fallback removal
from PR #3951, the GLOB-007 optional runtime handle fallback removal from
PR #3952, and the GLOB-007 AppContext-only fallback signature cleanup from
PR #3953.
- Current phase PR: GLOB-007 runtime resolver fallback boundary cleanup.
- Based on: `origin/main` after PR #3953 merged.
PR #3952, the GLOB-007 AppContext-only fallback signature cleanup from
PR #3953, and the GLOB-007 runtime resolver fallback boundary cleanup from
PR #3954.
- Current phase PR: GLOB-007 service/credential resolver fallback removal.
- Based on: current `origin/main` after PR #3954 merged.
- PR type for this branch: `ci-gate`.
- Runtime behavior changes: none intended; public runtime facades keep their
existing no-AppContext fallback behavior.
- Rust code changes: move remaining resolver helper fallback parameters to the
public facade boundary for KMS, IAM, TLS, metrics, S3 Select, node identity,
tier/expiry, credentials, runtime port, and buffer config reads.
- Runtime behavior changes: when no AppContext is published, service/credential
resolvers for KMS manager lookup, encryption service, IAM readiness/handle,
ready IAM handle, and action credentials now return their empty/error state
instead of consulting legacy globals.
- Rust code changes: remove those public no-AppContext fallbacks, delete the
now-unused legacy runtime-source adapters, and pass object ZIP download token
signing credentials explicitly after resolving them at handler boundaries.
SSE/KMS tests now inject their test KMS service manager explicitly instead of
relying on no-AppContext fallback reads.
- CI/script changes: none intended.
- Docs changes: update this progress ledger for the fallback boundary cleanup.
- Docs changes: update this progress ledger for the service/credential fallback
removal.
## Phase 0 Tasks
@@ -2826,6 +2832,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
- Current slice: move the remaining resolver helper fallback parameters to
public runtime facade boundaries, preserving no-AppContext fallback behavior
while making private helpers context-only.
- Current slice: remove the public no-AppContext legacy fallbacks from
service/credential resolvers for KMS manager lookup, encryption service,
IAM readiness/handle, ready IAM handle, and action credentials.
- Remaining work: remove the next fallback family per PR only after scans
prove no production caller depends on it.
- Verification: focused RustFS compile and admin test-target compile,
@@ -6094,6 +6103,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
| Expert | Status | Notes |
|---|---|---|
| Quality/architecture | pass | GLOB-007 removes the next cohesive service/credential fallback family without changing public resolver signatures or caller-side error handling contracts. |
| Migration preservation | pass | AppContext-backed behavior is unchanged; absent AppContext now returns `None`, `false`, or `IamSysNotInitialized` for the affected service/credential resolvers instead of consulting legacy globals. |
| Testing/verification | pass | Focused resolver, object ZIP download, and SSE/KMS tests passed; compile, formatting, architecture guard, service/credential fallback residual scan, diff hygiene, Rust risk scan, Rust quality scan, and full `make pre-pr` are required before PR. |
| Quality/architecture | pass | GLOB-007 moves the remaining resolver helper fallback parameters out to public facade boundaries, making private helper paths context-only without hiding fallback closures. |
| Migration preservation | pass | Public runtime facade names, return types, and no-AppContext fallback behavior are preserved; only helper signatures and focused resolver tests change. |
| Testing/verification | pass | Focused resolver test, compile, formatting, architecture guard, fallback-boundary residual scan, diff hygiene, diff-added Rust risk scan, Rust quality scan, and full `make pre-pr` passed before PR. |
@@ -6487,6 +6499,28 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
Passed before push:
- Issue #660 GLOB-007 service/credential resolver fallback removal:
- Branch freshness check: rebased onto current `origin/main` after PR #3954
merged.
- `cargo test -p rustfs --lib app::context::tests::resolver_helpers_are_context_first_and_empty_when_context_is_absent`:
passed.
- `cargo test -p rustfs --lib admin::handlers::object_zip_download::tests`:
passed.
- `cargo test -p rustfs --lib storage::sse::tests::test_kms_sse_dek_provider_uses_latest_reconfigured_service`:
passed.
- `cargo check -p rustfs --lib`: passed.
- `cargo fmt --all`: passed.
- `cargo fmt --all --check`: passed.
- `git diff --check`: passed.
- `./scripts/check_architecture_migration_rules.sh`: passed.
- Service/credential fallback residual scan: passed.
- Removed runtime-source fallback adapter scan: passed.
- Diff-added Rust risk scan: passed.
- Rust code quality scan: passed.
- Three-expert review: passed.
- `make pre-pr`: passed (`nextest`: 6917 passed, 112 skipped; doctests
passed).
- Issue #660 GLOB-007 runtime resolver fallback boundary cleanup:
- Branch freshness check: moved onto `origin/main` after PR #3953 merged.
- `cargo test -p rustfs --lib app::context::tests::resolver_helpers_are_context_first_and_empty_when_context_is_absent`:
@@ -34,6 +34,7 @@ use hyper::{Method, Uri};
use matchit::Params;
use rand::RngExt;
use rustfs_config::MAX_ADMIN_REQUEST_BODY_SIZE;
use rustfs_credentials::Credentials;
use rustfs_policy::policy::action::{Action, S3Action};
use rustfs_trusted_proxies::{ClientInfo, ValidationMode};
use rustfs_utils::{base64_decode_url_safe_no_pad, base64_encode_url_safe_no_pad};
@@ -281,9 +282,11 @@ impl From<ClientInfoSnapshot> for ClientInfo {
}
}
fn download_token_encryption_key() -> S3Result<[u8; 32]> {
let credentials =
current_action_credentials().ok_or_else(|| s3_error!(InternalError, "global action credentials are not initialized"))?;
fn current_download_token_credentials() -> S3Result<Credentials> {
current_action_credentials().ok_or_else(|| s3_error!(InternalError, "global action credentials are not initialized"))
}
fn download_token_encryption_key(credentials: &Credentials) -> S3Result<[u8; 32]> {
if credentials.secret_key.is_empty() {
return Err(s3_error!(InternalError, "global action credentials are not initialized"));
}
@@ -293,10 +296,10 @@ fn download_token_encryption_key() -> S3Result<[u8; 32]> {
Ok(hasher.finalize().into())
}
fn encode_download_token(payload: &ObjectZipDownloadTokenPayload) -> S3Result<String> {
fn encode_download_token(payload: &ObjectZipDownloadTokenPayload, credentials: &Credentials) -> S3Result<String> {
let payload_json =
serde_json::to_vec(payload).map_err(|e| s3_error!(InternalError, "failed to serialize download token: {}", e))?;
let key = download_token_encryption_key()?;
let key = download_token_encryption_key(credentials)?;
let cipher = Aes256Gcm::new(&Key::<Aes256Gcm>::from(key));
let mut nonce_bytes = [0_u8; 12];
rand::rng().fill(&mut nonce_bytes);
@@ -311,7 +314,7 @@ fn encode_download_token(payload: &ObjectZipDownloadTokenPayload) -> S3Result<St
))
}
fn decode_download_token(token: &str) -> S3Result<ObjectZipDownloadTokenPayload> {
fn decode_download_token(token: &str, credentials: &Credentials) -> S3Result<ObjectZipDownloadTokenPayload> {
let Some((nonce_part, ciphertext_part)) = token.split_once('.') else {
return Err(s3_error!(AccessDenied, "invalid or expired download token"));
};
@@ -327,7 +330,7 @@ fn decode_download_token(token: &str) -> S3Result<ObjectZipDownloadTokenPayload>
.as_slice()
.try_into()
.map_err(|_| s3_error!(AccessDenied, "invalid or expired download token"))?;
let key = download_token_encryption_key()?;
let key = download_token_encryption_key(credentials)?;
let cipher = Aes256Gcm::new(&Key::<Aes256Gcm>::from(key));
let payload = cipher
.decrypt(&Nonce::from(nonce), ciphertext.as_slice())
@@ -342,6 +345,7 @@ fn create_download_token(
auth_context: ObjectZipDownloadAuthContext,
request: CreateObjectZipDownloadRequest,
now: OffsetDateTime,
credentials: &Credentials,
) -> S3Result<CreatedObjectZipDownloadToken> {
let id = Uuid::new_v4().to_string();
let expires_at = now + OBJECT_ZIP_DOWNLOAD_TOKEN_TTL;
@@ -353,13 +357,18 @@ fn create_download_token(
request,
expires_at_unix: expires_at.unix_timestamp(),
};
let token = encode_download_token(&payload)?;
let token = encode_download_token(&payload, credentials)?;
Ok(CreatedObjectZipDownloadToken { id, token, expires_at })
}
fn validate_download_token(id: &str, token: &str, now: OffsetDateTime) -> S3Result<ObjectZipDownloadToken> {
let payload = decode_download_token(token)?;
fn validate_download_token(
id: &str,
token: &str,
now: OffsetDateTime,
credentials: &Credentials,
) -> S3Result<ObjectZipDownloadToken> {
let payload = decode_download_token(token, credentials)?;
if payload.id != id {
return Err(s3_error!(AccessDenied, "invalid or expired download token"));
}
@@ -555,7 +564,8 @@ impl Operation for CreateObjectZipDownloadHandler {
let principal = authenticated_zip_download_principal(&req)?;
let req_info = authenticated_zip_download_req_info(&req)?;
let auth_context = authenticated_zip_download_auth_context(&req);
let created = create_download_token(principal, req_info, auth_context, request, OffsetDateTime::now_utc())?;
let credentials = current_download_token_credentials()?;
let created = create_download_token(principal, req_info, auth_context, request, OffsetDateTime::now_utc(), &credentials)?;
build_json_response(
StatusCode::OK,
@@ -579,7 +589,8 @@ impl Operation for DownloadObjectZipHandler {
return Err(s3_error!(AccessDenied, "invalid or expired download token"));
}
let record = validate_download_token(id, &token, OffsetDateTime::now_utc())?;
let credentials = current_download_token_credentials()?;
let record = validate_download_token(id, &token, OffsetDateTime::now_utc(), &credentials)?;
let _token_id = &record.id;
let _principal = &record.principal;
let prepared = prepare_zip_download_archive(&record).await?;
@@ -899,7 +910,6 @@ struct ZipDownloadItem {
#[cfg(test)]
mod tests {
use super::*;
use rustfs_credentials::init_global_action_credentials;
use s3s::S3ErrorCode;
fn valid_request() -> CreateObjectZipDownloadRequest {
@@ -1008,7 +1018,8 @@ mod tests {
#[test]
fn validate_download_token_rejects_unknown_token() {
let err = validate_download_token("missing", "missing", OffsetDateTime::UNIX_EPOCH)
let credentials = test_signing_credentials();
let err = validate_download_token("missing", "missing", OffsetDateTime::UNIX_EPOCH, &credentials)
.expect_err("unknown token should be rejected");
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
@@ -1018,8 +1029,10 @@ mod tests {
fn validate_download_token_rejects_mismatched_token_value() {
let now = OffsetDateTime::UNIX_EPOCH;
let created = create_test_download_token(now);
let credentials = test_signing_credentials();
let err = validate_download_token("different-id", &created.token, now).expect_err("wrong token id should be rejected");
let err = validate_download_token("different-id", &created.token, now, &credentials)
.expect_err("wrong token id should be rejected");
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
}
@@ -1028,8 +1041,9 @@ mod tests {
fn validate_download_token_rejects_expired_token() {
let now = OffsetDateTime::UNIX_EPOCH;
let created = create_test_download_token(now);
let credentials = test_signing_credentials();
let err = validate_download_token(&created.id, &created.token, now + OBJECT_ZIP_DOWNLOAD_TOKEN_TTL)
let err = validate_download_token(&created.id, &created.token, now + OBJECT_ZIP_DOWNLOAD_TOKEN_TTL, &credentials)
.expect_err("expired token should be rejected");
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
@@ -1039,8 +1053,10 @@ mod tests {
fn validate_download_token_accepts_unexpired_matching_token() {
let now = OffsetDateTime::UNIX_EPOCH;
let created = create_test_download_token(now);
let credentials = test_signing_credentials();
let record = validate_download_token(&created.id, &created.token, now).expect("matching token should be accepted");
let record =
validate_download_token(&created.id, &created.token, now, &credentials).expect("matching token should be accepted");
assert_eq!(record.id, created.id);
assert_eq!(record.principal, "alice");
@@ -1077,8 +1093,10 @@ mod tests {
let first = ciphertext.first_mut().expect("ciphertext should not be empty");
*first ^= 0x01;
let tampered = format!("{}.{}", nonce, base64_encode_url_safe_no_pad(&ciphertext));
let credentials = test_signing_credentials();
let err = validate_download_token(&created.id, &tampered, now).expect_err("tampered token should be rejected");
let err =
validate_download_token(&created.id, &tampered, now, &credentials).expect_err("tampered token should be rejected");
assert_eq!(err.code(), &S3ErrorCode::AccessDenied);
}
@@ -1371,17 +1389,23 @@ mod tests {
}
fn create_test_download_token(now: OffsetDateTime) -> CreatedObjectZipDownloadToken {
ensure_test_signing_credentials();
create_download_token("alice".to_string(), test_req_info(), test_auth_context(), valid_request(), now)
.expect("token should be created")
let credentials = test_signing_credentials();
create_download_token(
"alice".to_string(),
test_req_info(),
test_auth_context(),
valid_request(),
now,
&credentials,
)
.expect("token should be created")
}
fn ensure_test_signing_credentials() {
if current_action_credentials().is_none() {
let _ = init_global_action_credentials(
Some("TESTROOTACCESSKEY".to_string()),
Some("TESTROOTSECRET1234567890".to_string()),
);
fn test_signing_credentials() -> Credentials {
Credentials {
access_key: "TESTROOTACCESSKEY".to_string(),
secret_key: "TESTROOTSECRET1234567890".to_string(),
..Default::default()
}
}
+5 -14
View File
@@ -48,7 +48,6 @@ use tokio::sync::RwLock;
/// Resolve KMS runtime service manager using AppContext-first precedence.
pub fn resolve_kms_runtime_service_manager() -> Option<Arc<KmsServiceManager>> {
resolve_kms_runtime_service_manager_with(get_global_app_context())
.or_else(|| default_kms_runtime_interface().service_manager())
}
/// Resolve or initialize the KMS runtime service manager using AppContext-first precedence.
@@ -59,11 +58,7 @@ pub fn resolve_or_init_kms_runtime_service_manager() -> Arc<KmsServiceManager> {
/// Resolve KMS encryption service using AppContext-first precedence.
pub async fn resolve_encryption_service() -> Option<Arc<ObjectEncryptionService>> {
if let Some(service) = resolve_encryption_service_with(get_global_app_context()).await {
return Some(service);
}
runtime_sources::encryption_service().await
resolve_encryption_service_with(get_global_app_context()).await
}
/// Resolve outbound TLS generation using AppContext-first precedence.
@@ -84,12 +79,12 @@ pub async fn resolve_outbound_tls_state() -> GlobalPublishedOutboundTlsState {
/// Resolve IAM readiness using AppContext-first precedence.
pub fn resolve_iam_ready() -> bool {
resolve_iam_ready_with(get_global_app_context()).unwrap_or_else(runtime_sources::iam_is_ready)
resolve_iam_ready_with(get_global_app_context()).unwrap_or(false)
}
/// Resolve IAM system handle using AppContext-first precedence.
pub fn resolve_iam_handle() -> Option<Arc<IamSys<ObjectStore>>> {
resolve_iam_handle_with(get_global_app_context()).or_else(runtime_sources::iam_handle)
resolve_iam_handle_with(get_global_app_context())
}
/// Resolve OIDC system handle using AppContext-first precedence.
@@ -108,7 +103,7 @@ pub fn resolve_ready_iam_handle() -> rustfs_iam::error::Result<Arc<IamSys<Object
return resolve_ready_iam_handle_with(context);
}
runtime_sources::ready_iam_handle()
Err(IamError::IamSysNotInitialized)
}
/// Resolve token signing key using AppContext-first precedence.
@@ -247,11 +242,7 @@ pub async fn resolve_local_node_name() -> String {
/// Resolve action credentials using AppContext-first precedence.
pub fn resolve_action_credentials() -> Option<Credentials> {
if let Some(context) = get_global_app_context() {
return resolve_action_credentials_with(context);
}
default_action_credential_interface().get()
get_global_app_context().and_then(resolve_action_credentials_with)
}
/// Resolve region using AppContext-first precedence.
+1 -13
View File
@@ -30,7 +30,7 @@ use rustfs_io_metrics::{
global_metrics::get_global_metrics,
internode_metrics::{InternodeMetrics, global_internode_metrics},
};
use rustfs_kms::{KmsServiceManager, ObjectEncryptionService};
use rustfs_kms::KmsServiceManager;
use rustfs_lock::LockClient;
use rustfs_notify::{EventArgs, NotificationError, notifier_global};
use rustfs_s3select_api::{QueryResult, server::dbms::DatabaseManagerSystem};
@@ -50,10 +50,6 @@ pub fn init_kms_service_manager() -> Arc<KmsServiceManager> {
rustfs_kms::init_global_kms_service_manager()
}
pub async fn encryption_service() -> Option<Arc<ObjectEncryptionService>> {
rustfs_kms::get_global_encryption_service().await
}
pub fn outbound_tls_generation() -> TlsGeneration {
load_global_outbound_tls_generation()
}
@@ -67,14 +63,6 @@ pub async fn outbound_tls_state() -> GlobalPublishedOutboundTlsState {
load_global_outbound_tls_state().await
}
pub fn iam_is_ready() -> bool {
rustfs_iam::get_global_iam_sys().is_some_and(|sys| sys.is_ready())
}
pub fn iam_handle() -> Option<Arc<IamSys<ObjectStore>>> {
rustfs_iam::get_global_iam_sys()
}
pub fn ready_iam_handle() -> IamResult<Arc<IamSys<ObjectStore>>> {
rustfs_iam::get()
}
+38 -8
View File
@@ -1792,18 +1792,43 @@ pub trait SseDekProvider: Send + Sync {
/// Production KMS-backed DEK provider
/// Resolves the latest ObjectEncryptionService on each call.
struct KmsSseDekProvider;
struct KmsSseDekProvider {
#[cfg(test)]
service_manager: Option<Arc<rustfs_kms::KmsServiceManager>>,
}
impl KmsSseDekProvider {
/// Create a new KMS-backed provider
pub async fn new() -> Result<Self, ApiError> {
Self::current_service()
let provider = Self {
#[cfg(test)]
service_manager: None,
};
provider
.current_service()
.await
.ok_or_else(|| ApiError::from(StorageError::other("KMS encryption service is not initialized")))?;
Ok(Self)
Ok(provider)
}
async fn current_service() -> Option<Arc<rustfs_kms::service::ObjectEncryptionService>> {
#[cfg(test)]
async fn new_with_service_manager(service_manager: Arc<rustfs_kms::KmsServiceManager>) -> Result<Self, ApiError> {
let provider = Self {
service_manager: Some(service_manager),
};
provider
.current_service()
.await
.ok_or_else(|| ApiError::from(StorageError::other("KMS encryption service is not initialized")))?;
Ok(provider)
}
async fn current_service(&self) -> Option<Arc<rustfs_kms::service::ObjectEncryptionService>> {
#[cfg(test)]
if let Some(service_manager) = &self.service_manager {
return service_manager.get_encryption_service().await;
}
runtime_sources::current_encryption_service().await
}
}
@@ -1816,7 +1841,8 @@ impl SseDekProvider for KmsSseDekProvider {
kms_key_id: &str,
) -> Result<(DataKey, Vec<u8>), ApiError> {
let kms_key_option = Some(kms_key_id.to_string());
let service = Self::current_service()
let service = self
.current_service()
.await
.ok_or_else(|| ApiError::from(StorageError::other("KMS encryption service is not initialized")))?;
let (data_key, encrypted_data_key) = service
@@ -1833,7 +1859,8 @@ impl SseDekProvider for KmsSseDekProvider {
_kms_key_id: &str,
context: &ObjectEncryptionContext,
) -> Result<[u8; 32], ApiError> {
let service = Self::current_service()
let service = self
.current_service()
.await
.ok_or_else(|| ApiError::from(StorageError::other("KMS encryption service is not initialized")))?;
let data_key = service
@@ -1851,7 +1878,8 @@ impl SseDekProvider for KmsSseDekProvider {
_kms_key_id: &str,
_context: &ObjectEncryptionContext,
) -> Result<[u8; 32], ApiError> {
let service = Self::current_service()
let service = self
.current_service()
.await
.ok_or_else(|| ApiError::from(StorageError::other("KMS encryption service is not initialized")))?;
let data_key = service
@@ -3725,7 +3753,9 @@ mod tests {
.await
.expect("first key should be created");
let provider = KmsSseDekProvider::new().await.expect("provider should initialize");
let provider = KmsSseDekProvider::new_with_service_manager(manager.clone())
.await
.expect("provider should initialize");
let context = ObjectEncryptionContext::new("bucket".to_string(), "object".to_string());
provider
.generate_sse_dek(&context, "first-key")