From bc83020d1942dcc8b7fa6c29a296f2eb8fe99ba9 Mon Sep 17 00:00:00 2001 From: Harald Gutmann Date: Sat, 7 Sep 2024 20:06:23 +0200 Subject: [PATCH] fix: remove indirection in connectors:tls:Connector replace direct field access with getter method that returns &Arc, store concrete type in now private struct field using impl Trait in return value keeps method signature stable across features; allows for static dispatch adjust visibility for TlsConnectorCtx structs from pub(crate) to pub(super) --- pingora-core/src/connectors/mod.rs | 12 ++++---- .../connectors/tls/boringssl_openssl/mod.rs | 7 +++-- pingora-core/src/connectors/tls/mod.rs | 28 +++++++++++++------ pingora-core/src/connectors/tls/rustls/mod.rs | 7 +++-- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/pingora-core/src/connectors/mod.rs b/pingora-core/src/connectors/mod.rs index b3078fdf..38da0cd9 100644 --- a/pingora-core/src/connectors/mod.rs +++ b/pingora-core/src/connectors/mod.rs @@ -174,11 +174,13 @@ impl TransportConnector { let stream = if let Some(rt) = rt { let peer = peer.clone(); let tls_ctx = self.tls_ctx.clone(); - rt.spawn(async move { do_connect(&peer, bind_to, alpn_override, &tls_ctx.ctx).await }) - .await - .or_err(InternalError, "offload runtime failure")?? + rt.spawn( + async move { do_connect(&peer, bind_to, alpn_override, tls_ctx.context()).await }, + ) + .await + .or_err(InternalError, "offload runtime failure")?? } else { - do_connect(peer, bind_to, alpn_override, &self.tls_ctx.ctx).await? + do_connect(peer, bind_to, alpn_override, self.tls_ctx.context()).await? }; Ok(stream) @@ -437,7 +439,7 @@ mod tests { /// the decomposed error type and message async fn get_do_connect_failure_with_peer(peer: &BasicPeer) -> (ErrorType, String) { let connector = Connector::new(None); - let stream = do_connect(peer, None, None, &connector.ctx).await; + let stream = do_connect(peer, None, None, connector.context()).await; match stream { Ok(_) => panic!("should throw an error"), Err(e) => ( diff --git a/pingora-core/src/connectors/tls/boringssl_openssl/mod.rs b/pingora-core/src/connectors/tls/boringssl_openssl/mod.rs index dd3dfc33..a16c61c5 100644 --- a/pingora-core/src/connectors/tls/boringssl_openssl/mod.rs +++ b/pingora-core/src/connectors/tls/boringssl_openssl/mod.rs @@ -94,7 +94,7 @@ fn init_ssl_cert_env_vars() { INIT_CA_ENV.call_once(openssl_probe::init_ssl_cert_env_vars); } -pub(crate) struct TlsConnectorCtx(pub(crate) SslConnector); +pub(super) struct TlsConnectorCtx(SslConnector); impl TlsConnectorContext for TlsConnectorCtx { fn as_any(&self) -> &dyn Any { @@ -162,15 +162,16 @@ impl TlsConnectorContext for TlsConnectorCtx { } } -pub(super) async fn connect( +pub(super) async fn connect( stream: T, peer: &P, alpn_override: Option, - tls_ctx: &Arc, + tls_ctx: &Arc, ) -> Result> where T: IO, P: Peer + Send + Sync, + C: TlsConnectorContext + Send + Sync, { let ctx = tls_ctx.as_any().downcast_ref::().unwrap(); let mut ssl_conf = ctx.0.configure().unwrap(); diff --git a/pingora-core/src/connectors/tls/mod.rs b/pingora-core/src/connectors/tls/mod.rs index 01167850..9ede8049 100644 --- a/pingora-core/src/connectors/tls/mod.rs +++ b/pingora-core/src/connectors/tls/mod.rs @@ -40,16 +40,20 @@ pub(crate) mod rustls; #[derive(Clone)] pub struct Connector { - pub(crate) ctx: Arc, // Arc to support clone + ctx: Arc, // Arc to support clone } impl Connector { pub fn new(options: Option) -> Self { TlsConnectorCtx::build_connector(options) } + + pub fn context(&self) -> &Arc { + &self.ctx + } } -pub(crate) trait TlsConnectorContext { +pub trait TlsConnectorContext { fn as_any(&self) -> &dyn Any; fn build_connector(options: Option) -> Connector @@ -57,12 +61,16 @@ pub(crate) trait TlsConnectorContext { Self: Sized; } -pub(super) async fn do_connect( +pub(super) async fn do_connect( peer: &P, bind_to: Option, alpn_override: Option, - tls_ctx: &Arc, -) -> Result { + tls_ctx: &Arc, +) -> Result +where + P: Peer + Send + Sync, + C: TlsConnectorContext + Send + Sync, +{ // Create the future that does the connections, but don't evaluate it until // we decide if we need a timeout or not let connect_future = do_connect_inner(peer, bind_to, alpn_override, tls_ctx); @@ -79,12 +87,16 @@ pub(super) async fn do_connect( } } -async fn do_connect_inner( +async fn do_connect_inner( peer: &P, bind_to: Option, alpn_override: Option, - tls_ctx: &Arc, -) -> Result { + tls_ctx: &Arc, +) -> Result +where + P: Peer + Send + Sync, + C: TlsConnectorContext + Send + Sync, +{ let stream = l4_connect(peer, bind_to).await?; if peer.tls() { let tls_stream = tls_connect(stream, peer, alpn_override, tls_ctx).await?; diff --git a/pingora-core/src/connectors/tls/rustls/mod.rs b/pingora-core/src/connectors/tls/rustls/mod.rs index 453c0ab7..9953e7a3 100644 --- a/pingora-core/src/connectors/tls/rustls/mod.rs +++ b/pingora-core/src/connectors/tls/rustls/mod.rs @@ -41,7 +41,7 @@ use crate::upstreams::peer::Peer; use super::{replace_leftmost_underscore, Connector, TlsConnectorContext}; -pub(crate) struct TlsConnectorCtx { +pub(super) struct TlsConnectorCtx { config: RusTlsClientConfig, ca_certs: RootCertStore, } @@ -110,15 +110,16 @@ impl TlsConnectorContext for TlsConnectorCtx { } } -pub(super) async fn connect( +pub(super) async fn connect( stream: T, peer: &P, alpn_override: Option, - tls_ctx: &Arc, + tls_ctx: &Arc, ) -> Result> where T: IO, P: Peer + Send + Sync, + C: TlsConnectorContext + Send + Sync, { let ctx = tls_ctx.as_any().downcast_ref::().unwrap(); let mut config = ctx.config.clone();