From 6eb238ce75b124badec1e4341324ec52e945448e Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 10 Jul 2026 00:00:47 +0800 Subject: [PATCH] refactor(app): give each context's credential handle its own copy (#4627) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backlog#1052 S3, fifth slice (also part of what the plan called S4). ActionCredentialHandle was a unit struct forwarding get to the process singleton (rustfs_credentials' GLOBAL_ACTIVE_CRED), so every context served the same credentials regardless of which server it belonged to. The handle now keeps its own copy and gains publish(): - publish() lands on the owning context and tries to publish the process global; readers prefer the owned copy and fall back to the global while the initial startup publish still lands there — ambient callers (iam token signing, request-signature validation) keep working. - Single instance: both cells are written together, so reads are unchanged; two contexts that both published stay isolated even though the global remembers only the first (covered by a new test). - The publish return signals "took effect": true if this handle newly holds credentials OR the global was newly initialized, matching the pre-existing init_global_action_credentials fail-fast contract, now scoped to the handle. The interface trait gains a default-body-friendly signature; the test double implements it as a no-op. The startup publish path (S4) still calls init_global_action_credentials directly; wiring it through the handle is a follow-up. --- rustfs/src/app/context.rs | 4 ++ rustfs/src/app/context/handles.rs | 66 +++++++++++++++++++++++++++- rustfs/src/app/context/interfaces.rs | 9 ++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/rustfs/src/app/context.rs b/rustfs/src/app/context.rs index c919ab0b3..4f6eebf7c 100644 --- a/rustfs/src/app/context.rs +++ b/rustfs/src/app/context.rs @@ -783,6 +783,10 @@ mod tests { fn get(&self) -> Option { self.credentials.clone() } + + fn publish(&self, _credentials: Credentials) -> bool { + false + } } struct TestRegionInterface { diff --git a/rustfs/src/app/context/handles.rs b/rustfs/src/app/context/handles.rs index 55858d09f..b2b864eec 100644 --- a/rustfs/src/app/context/handles.rs +++ b/rustfs/src/app/context/handles.rs @@ -344,13 +344,38 @@ impl LocalNodeNameInterface for LocalNodeNameHandle { } /// Default action credentials interface adapter. +/// +/// Holds this context's own copy of the action credentials (backlog#1052 S3). +/// `publish` lands on the owning context *and* attempts to publish the process +/// global; readers prefer the owned copy and fall back to the global — so +/// ambient callers (iam, S3 auth) keep working, and two contexts that both +/// published stay isolated even though the global remembers only the first. #[derive(Default)] -pub struct ActionCredentialHandle; +pub struct ActionCredentialHandle { + credentials: StdRwLock>, +} impl ActionCredentialInterface for ActionCredentialHandle { fn get(&self) -> Option { + if let Ok(guard) = self.credentials.read() + && let Some(credentials) = guard.as_ref() + { + return Some(credentials.clone()); + } runtime_sources::action_credentials() } + + fn publish(&self, credentials: Credentials) -> bool { + let Ok(mut guard) = self.credentials.write() else { + return false; + }; + let already_held = guard.is_some(); + *guard = Some(credentials.clone()); + let global_published = + rustfs_credentials::init_global_action_credentials(Some(credentials.access_key), Some(credentials.secret_key)) + .is_ok(); + !already_held || global_published + } } /// Default region interface adapter. @@ -528,7 +553,7 @@ pub fn default_local_node_name_interface() -> Arc { } pub fn default_action_credential_interface() -> Arc { - Arc::new(ActionCredentialHandle) + Arc::new(ActionCredentialHandle::default()) } pub fn default_oidc_interface() -> Arc { @@ -600,4 +625,41 @@ mod tests { let handle = ServerConfigHandle::default(); assert_eq!(handle.get(), runtime_sources::server_config()); } + + // backlog#1052 S3: two contexts' credential handles keep their own copies, + // so they stay isolated even though the process global is init-once and + // will only remember the first publish. + #[test] + fn credential_handle_prefers_its_own_copy_over_the_shared_global() { + use super::ActionCredentialHandle; + use crate::app::context::interfaces::ActionCredentialInterface; + use rustfs_credentials::Credentials; + + let cred_a = Credentials { + access_key: "handle-a-access".to_string(), + secret_key: "handle-a-secret".to_string(), + ..Default::default() + }; + let cred_b = Credentials { + access_key: "handle-b-access".to_string(), + secret_key: "handle-b-secret".to_string(), + ..Default::default() + }; + + let handle_a = ActionCredentialHandle::default(); + let handle_b = ActionCredentialHandle::default(); + handle_a.publish(cred_a.clone()); + handle_b.publish(cred_b.clone()); + + assert_eq!( + handle_a.get().map(|c| c.access_key), + Some(cred_a.access_key), + "handle A must serve its own credentials" + ); + assert_eq!( + handle_b.get().map(|c| c.access_key), + Some(cred_b.access_key), + "handle B must serve its own credentials" + ); + } } diff --git a/rustfs/src/app/context/interfaces.rs b/rustfs/src/app/context/interfaces.rs index 6c580f522..728dd1b11 100644 --- a/rustfs/src/app/context/interfaces.rs +++ b/rustfs/src/app/context/interfaces.rs @@ -175,6 +175,15 @@ pub trait LocalNodeNameInterface: Send + Sync { /// Action credentials interface for admin handler integration. pub trait ActionCredentialInterface: Send + Sync { fn get(&self) -> Option; + + /// Publish this context's action credentials (backlog#1052 S3). + /// + /// Returns `false` if the process global was already initialized *and* + /// this handle already held credentials — matching the pre-existing + /// fail-fast contract of `rustfs_credentials::init_global_action_credentials`, + /// now scoped to the handle. Single-instance: the first server wins and + /// subsequent no-op or return false, exactly as before. + fn publish(&self, credentials: Credentials) -> bool; } /// Region interface for application-layer use-cases.