refactor(connlib): extract ConnectArgs (#5488)

This is extracted from #5487 since I needed to add an 8th parameter and
Clippy said 8 is too many.

Refs #2986 
Stepping stone towards using the Builder pattern. There's only a few
Clients so this has 80% of the advantage for 20% of the effort
This commit is contained in:
Reactor Scram
2024-06-21 20:58:47 +00:00
committed by GitHub
parent 53c74ae094
commit 15ad02e45f
4 changed files with 65 additions and 66 deletions

View File

@@ -4,8 +4,8 @@
// ecosystem, so it's used here for consistency.
use connlib_client_shared::{
callbacks::ResourceDescription, file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error,
LoginUrl, LoginUrlError, Session, Sockets,
callbacks::ResourceDescription, file_logger, keypair, Callbacks, Cidrv4, Cidrv6, ConnectArgs,
Error, LoginUrl, LoginUrlError, Session, Sockets,
};
use jni::{
objects::{GlobalRef, JClass, JObject, JString, JValue},
@@ -346,14 +346,14 @@ fn connect(
let handle = init_logging(&PathBuf::from(log_dir), log_filter);
let callback_handler = CallbackHandler {
let callbacks = CallbackHandler {
vm: env.get_java_vm().map_err(ConnectError::GetJavaVmFailed)?,
callback_handler,
handle,
};
let (private_key, public_key) = keypair();
let login = LoginUrl::client(
let url = LoginUrl::client(
api_url.as_str(),
&secret,
device_id,
@@ -368,7 +368,7 @@ fn connect(
.build()?;
let sockets = Sockets::with_protect({
let callbacks = callback_handler.clone();
let callbacks = callbacks.clone();
move |fd| {
callbacks
.protect_file_descriptor(fd)
@@ -376,15 +376,15 @@ fn connect(
}
});
let session = Session::connect(
login,
let args = ConnectArgs {
url,
sockets,
private_key,
Some(os_version),
callback_handler,
Some(MAX_PARTITION_TIME),
runtime.handle().clone(),
);
os_version_override: Some(os_version),
callbacks,
max_partition_time: Some(MAX_PARTITION_TIME),
};
let session = Session::connect(args, runtime.handle().clone());
Ok(SessionWrapper {
inner: session,

View File

@@ -2,8 +2,8 @@
#![allow(clippy::unnecessary_cast, improper_ctypes, non_camel_case_types)]
use connlib_client_shared::{
callbacks::ResourceDescription, file_logger, keypair, Callbacks, Cidrv4, Cidrv6, Error,
LoginUrl, Session, Sockets,
callbacks::ResourceDescription, file_logger, keypair, Callbacks, Cidrv4, Cidrv6, ConnectArgs,
Error, LoginUrl, Session, Sockets,
};
use secrecy::SecretString;
use std::{
@@ -175,7 +175,7 @@ impl WrappedSession {
let secret = SecretString::from(token);
let (private_key, public_key) = keypair();
let login = LoginUrl::client(
let url = LoginUrl::client(
api_url.as_str(),
&secret,
device_id,
@@ -191,17 +191,17 @@ impl WrappedSession {
.build()
.map_err(|e| e.to_string())?;
let session = Session::connect(
login,
Sockets::new(),
let args = ConnectArgs {
url,
sockets: Sockets::new(),
private_key,
os_version_override,
CallbackHandler {
callbacks: CallbackHandler {
inner: Arc::new(callback_handler),
},
Some(MAX_PARTITION_TIME),
runtime.handle().clone(),
);
max_partition_time: Some(MAX_PARTITION_TIME),
};
let session = Session::connect(args, runtime.handle().clone());
Ok(Self {
inner: session,

View File

@@ -32,30 +32,28 @@ pub struct Session {
channel: tokio::sync::mpsc::UnboundedSender<Command>,
}
/// Arguments for `connect`, since Clippy said 8 args is too many
pub struct ConnectArgs<CB> {
pub url: LoginUrl,
pub sockets: Sockets,
pub private_key: StaticSecret,
pub os_version_override: Option<String>,
pub callbacks: CB,
pub max_partition_time: Option<Duration>,
}
impl Session {
/// Creates a new [`Session`].
///
/// This connects to the portal a specified using [`LoginUrl`] and creates a wireguard tunnel using the provided private key.
pub fn connect<CB: Callbacks + 'static>(
url: LoginUrl,
sockets: Sockets,
private_key: StaticSecret,
os_version_override: Option<String>,
callbacks: CB,
max_partition_time: Option<Duration>,
args: ConnectArgs<CB>,
handle: tokio::runtime::Handle,
) -> Self {
let (tx, rx) = tokio::sync::mpsc::unbounded_channel();
let connect_handle = handle.spawn(connect(
url,
sockets,
private_key,
os_version_override,
callbacks.clone(),
max_partition_time,
rx,
));
let callbacks = args.callbacks.clone();
let connect_handle = handle.spawn(connect(args, rx));
handle.spawn(connect_supervisor(connect_handle, callbacks));
Self { channel: tx }
@@ -106,19 +104,20 @@ impl Session {
/// Connects to the portal and starts a tunnel.
///
/// When this function exits, the tunnel failed unrecoverably and you need to call it again.
async fn connect<CB>(
url: LoginUrl,
sockets: Sockets,
private_key: StaticSecret,
os_version_override: Option<String>,
callbacks: CB,
max_partition_time: Option<Duration>,
rx: UnboundedReceiver<Command>,
) -> Result<(), Error>
async fn connect<CB>(args: ConnectArgs<CB>, rx: UnboundedReceiver<Command>) -> Result<(), Error>
where
CB: Callbacks + 'static,
{
let tunnel = ClientTunnel::new(private_key, sockets, callbacks.clone())?;
let ConnectArgs {
url,
sockets,
private_key,
os_version_override,
callbacks,
max_partition_time,
} = args;
let tunnel = ClientTunnel::new(private_key, sockets, callbacks)?;
let portal = PhoenixChannel::connect(
Secret::new(url),

View File

@@ -11,7 +11,7 @@
use anyhow::{anyhow, bail, Context as _, Result};
use clap::Parser;
use connlib_client_shared::{
file_logger, keypair, Callbacks, Error as ConnlibError, LoginUrl, Session, Sockets,
file_logger, keypair, Callbacks, ConnectArgs, Error as ConnlibError, LoginUrl, Session, Sockets,
};
use connlib_shared::{callbacks, tun_device_manager, Cidrv4, Cidrv6};
use firezone_cli_utils::setup_global_subscriber;
@@ -271,7 +271,7 @@ pub fn run_only_headless_client() -> Result<()> {
};
let (private_key, public_key) = keypair();
let login = LoginUrl::client(
let url = LoginUrl::client(
cli.api_url,
&token,
firezone_id,
@@ -285,18 +285,18 @@ pub fn run_only_headless_client() -> Result<()> {
}
let (cb_tx, mut cb_rx) = mpsc::channel(10);
let callback_handler = CallbackHandler { cb_tx };
let callbacks = CallbackHandler { cb_tx };
platform::setup_before_connlib()?;
let session = Session::connect(
login,
Sockets::new(),
let args = ConnectArgs {
url,
sockets: Sockets::new(),
private_key,
None,
callback_handler,
os_version_override: None,
callbacks,
max_partition_time,
rt.handle().clone(),
);
};
let session = Session::connect(args, rt.handle().clone());
// TODO: this should be added dynamically
session.set_dns(dns_control::system_resolvers().unwrap_or_default());
platform::notify_service_controller()?;
@@ -564,7 +564,7 @@ impl Handler {
device_id::get_or_create().context("Failed to get / create device ID")?;
let (private_key, public_key) = keypair();
let login = LoginUrl::client(
let url = LoginUrl::client(
Url::parse(&api_url)?,
&token,
device_id.id,
@@ -573,15 +573,15 @@ impl Handler {
)?;
self.last_connlib_start_instant = Some(Instant::now());
let new_session = connlib_client_shared::Session::connect(
login,
Sockets::new(),
let args = ConnectArgs {
url,
sockets: Sockets::new(),
private_key,
None,
self.callback_handler.clone(),
Some(Duration::from_secs(60 * 60 * 24 * 30)),
tokio::runtime::Handle::try_current()?,
);
os_version_override: None,
callbacks: self.callback_handler.clone(),
max_partition_time: Some(Duration::from_secs(60 * 60 * 24 * 30)),
};
let new_session = Session::connect(args, tokio::runtime::Handle::try_current()?);
new_session.set_dns(dns_control::system_resolvers().unwrap_or_default());
self.connlib = Some(new_session);
}