diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index eebfd34e6..b64718692 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -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`: diff --git a/rustfs/src/admin/handlers/object_zip_download.rs b/rustfs/src/admin/handlers/object_zip_download.rs index 011f5c67b..5cec480bb 100644 --- a/rustfs/src/admin/handlers/object_zip_download.rs +++ b/rustfs/src/admin/handlers/object_zip_download.rs @@ -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 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 { + 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 { +fn encode_download_token(payload: &ObjectZipDownloadTokenPayload, credentials: &Credentials) -> S3Result { 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::::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 S3Result { +fn decode_download_token(token: &str, credentials: &Credentials) -> S3Result { 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 .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::::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 { 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 { - let payload = decode_download_token(token)?; +fn validate_download_token( + id: &str, + token: &str, + now: OffsetDateTime, + credentials: &Credentials, +) -> S3Result { + 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() } } diff --git a/rustfs/src/app/context.rs b/rustfs/src/app/context.rs index 12432fe60..8579f1e91 100644 --- a/rustfs/src/app/context.rs +++ b/rustfs/src/app/context.rs @@ -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> { 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 { /// Resolve KMS encryption service using AppContext-first precedence. pub async fn resolve_encryption_service() -> Option> { - 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>> { - 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 String { /// Resolve action credentials using AppContext-first precedence. pub fn resolve_action_credentials() -> Option { - 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. diff --git a/rustfs/src/app/context/runtime_sources.rs b/rustfs/src/app/context/runtime_sources.rs index 16d4f4c0f..37d18b290 100644 --- a/rustfs/src/app/context/runtime_sources.rs +++ b/rustfs/src/app/context/runtime_sources.rs @@ -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 { rustfs_kms::init_global_kms_service_manager() } -pub async fn encryption_service() -> Option> { - 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>> { - rustfs_iam::get_global_iam_sys() -} - pub fn ready_iam_handle() -> IamResult>> { rustfs_iam::get() } diff --git a/rustfs/src/storage/sse.rs b/rustfs/src/storage/sse.rs index 4a7cf46c0..edc529d41 100644 --- a/rustfs/src/storage/sse.rs +++ b/rustfs/src/storage/sse.rs @@ -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>, +} impl KmsSseDekProvider { /// Create a new KMS-backed provider pub async fn new() -> Result { - 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> { + #[cfg(test)] + async fn new_with_service_manager(service_manager: Arc) -> Result { + 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> { + #[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), 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")