From 750ed33fad74bf79911104f60d2df3f5888a6d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Augusto=20C=C3=A9sar?= Date: Mon, 20 Nov 2023 10:55:19 +0100 Subject: [PATCH] refactor: shuffle code and responsibility of LocalState (#49) ### Description This changes make so that we stop using methods from the `start` command around the code. To achieve this, the responsibility of loading and saving the state is now part of the implementation of the `LocalState` itself. ### Other notable changes - Instead of passing around a Tuple for the local and remote `StorableSession`, now there is a `ServerConfig` which holds a `local` and `remote` fields. Where this struct is located could be moved, but didn't gave that much thought. --- linkup-cli/src/background_booting.rs | 117 +++++++++++++++------------ linkup-cli/src/local_config.rs | 40 ++++++++- linkup-cli/src/remote_local.rs | 18 ++--- linkup-cli/src/reset.rs | 6 +- linkup-cli/src/start.rs | 46 ++--------- linkup-cli/src/status.rs | 3 +- linkup-cli/src/stop.rs | 4 +- linkup/src/session.rs | 2 +- 8 files changed, 126 insertions(+), 110 deletions(-) diff --git a/linkup-cli/src/background_booting.rs b/linkup-cli/src/background_booting.rs index 38d4a67..cf929bb 100644 --- a/linkup-cli/src/background_booting.rs +++ b/linkup-cli/src/background_booting.rs @@ -11,14 +11,13 @@ use crate::background_local_server::{ }; use crate::background_tunnel::start_tunnel; use crate::local_config::{LocalState, ServiceTarget}; -use crate::start::save_state; use crate::status::print_session_names; use crate::worker_client::WorkerClient; +use crate::CliError; use crate::LINKUP_LOCALSERVER_PORT; -use crate::{start::get_state, CliError}; pub fn boot_background_services() -> Result<(), CliError> { - let mut state = get_state()?; + let mut state = LocalState::load()?; let local_url = Url::parse(&format!("http://localhost:{}", LINKUP_LOCALSERVER_PORT)) .expect("linkup url invalid"); @@ -40,35 +39,34 @@ pub fn boot_background_services() -> Result<(), CliError> { println!("Cloudflare tunnel was already running.. Try stopping linkup first if you have problems."); } - let (local_server_conf, remote_server_conf) = server_config_from_state(&state); + let server_config = ServerConfig::from(&state); let server_session_name = load_config( &state.linkup.remote, &state.linkup.session_name, - remote_server_conf, + server_config.remote, )?; - let local_session_name = load_config(&local_url, &server_session_name, local_server_conf)?; + let local_session_name = load_config(&local_url, &server_session_name, server_config.local)?; if server_session_name != local_session_name { return Err(CliError::InconsistentState); } - let tunnel_url = state.linkup.tunnel.clone(); + state.linkup.session_name = server_session_name; + state.save()?; - state.linkup.session_name = server_session_name.clone(); - let state_to_print = state.clone(); - - save_state(state)?; - - println!("Waiting for tunnel to be ready at {}...", tunnel_url); + println!( + "Waiting for tunnel to be ready at {}...", + &state.linkup.tunnel + ); // If the tunnel is checked too quickly, it dies ¯\_(ツ)_/¯ thread::sleep(Duration::from_millis(1000)); - wait_till_ok(format!("{}linkup-check", tunnel_url))?; + wait_till_ok(format!("{}linkup-check", &state.linkup.tunnel))?; println!(); - print_session_names(&state_to_print); + print_session_names(&state); Ok(()) } @@ -80,7 +78,7 @@ pub fn load_config( ) -> Result { let session_update_req = UpdateSessionRequest { session_token: config.session_token, - desired_name: desired_name.into(), + desired_name: desired_name.to_string(), services: config.services, domains: config.domains, cache_routes: config.cache_routes, @@ -93,49 +91,66 @@ pub fn load_config( Ok(content) } -pub fn server_config_from_state(state: &LocalState) -> (StorableSession, StorableSession) { - let local_server_services = state - .services - .iter() - .map(|local_service| StorableService { - name: local_service.name.clone(), - location: if local_service.current == ServiceTarget::Remote { - local_service.remote.clone() - } else { - local_service.local.clone() - }, - rewrites: Some(local_service.rewrites.clone()), - }) - .collect::>(); - - let remote_server_services = state - .services - .iter() - .map(|local_service| StorableService { - name: local_service.name.clone(), - location: if local_service.current == ServiceTarget::Remote { - local_service.remote.clone() - } else { - state.linkup.tunnel.clone() - }, - rewrites: Some(local_service.rewrites.clone()), - }) - .collect::>(); - - ( - StorableSession { +pub struct ServerConfig { + pub local: StorableSession, + pub remote: StorableSession, +} + +impl From<&LocalState> for ServerConfig { + fn from(state: &LocalState) -> Self { + let local_server_services = state + .services + .iter() + .map(|service| StorableService { + name: service.name.clone(), + location: if service.current == ServiceTarget::Remote { + service.remote.clone() + } else { + service.local.clone() + }, + rewrites: Some(service.rewrites.clone()), + }) + .collect::>(); + + let remote_server_services = state + .services + .iter() + .map(|service| StorableService { + name: service.name.clone(), + location: if service.current == ServiceTarget::Remote { + service.remote.clone() + } else { + state.linkup.tunnel.clone() + }, + rewrites: Some(service.rewrites.clone()), + }) + .collect::>(); + + let local_storable_session = StorableSession { session_token: state.linkup.session_token.clone(), services: local_server_services, domains: state.domains.clone(), cache_routes: state.linkup.cache_routes.clone(), - }, - StorableSession { + }; + + let remote_storable_session = StorableSession { session_token: state.linkup.session_token.clone(), services: remote_server_services, domains: state.domains.clone(), cache_routes: state.linkup.cache_routes.clone(), - }, - ) + }; + + ServerConfig { + local: local_storable_session, + remote: remote_storable_session, + } + } +} + +impl<'a> From<&'a ServerConfig> for (&'a StorableSession, &'a StorableSession) { + fn from(config: &'a ServerConfig) -> Self { + (&config.local, &config.remote) + } } pub fn wait_till_ok(url: String) -> Result<(), CliError> { diff --git a/linkup-cli/src/local_config.rs b/linkup-cli/src/local_config.rs index 6da49a9..736ab79 100644 --- a/linkup-cli/src/local_config.rs +++ b/linkup-cli/src/local_config.rs @@ -10,7 +10,7 @@ use url::Url; use linkup::{CreatePreviewRequest, StorableDomain, StorableRewrite, StorableService}; -use crate::{CliError, LINKUP_CONFIG_ENV}; +use crate::{linkup_file_path, CliError, LINKUP_CONFIG_ENV, LINKUP_STATE_FILE}; #[derive(Deserialize, Serialize, Clone)] pub struct LocalState { @@ -19,6 +19,44 @@ pub struct LocalState { pub services: Vec, } +impl LocalState { + pub fn load() -> Result { + if let Err(e) = fs::File::open(linkup_file_path(LINKUP_STATE_FILE)) { + return Err(CliError::NoState(e.to_string())); + } + + let content = match fs::read_to_string(linkup_file_path(LINKUP_STATE_FILE)) { + Ok(content) => content, + Err(e) => return Err(CliError::NoState(e.to_string())), + }; + + match serde_yaml::from_str(&content) { + Ok(config) => Ok(config), + Err(e) => Err(CliError::NoState(e.to_string())), + } + } + + pub fn save(&mut self) -> Result<(), CliError> { + let yaml_string = match serde_yaml::to_string(self) { + Ok(yaml) => yaml, + Err(_) => { + return Err(CliError::SaveState( + "Failed to serialize the state into YAML".to_string(), + )) + } + }; + + if fs::write(linkup_file_path(LINKUP_STATE_FILE), yaml_string).is_err() { + return Err(CliError::SaveState(format!( + "Failed to write the state file at {}", + linkup_file_path(LINKUP_STATE_FILE).display() + ))); + } + + Ok(()) + } +} + #[derive(Deserialize, Serialize, Clone)] pub struct LinkupState { pub session_name: String, diff --git a/linkup-cli/src/remote_local.rs b/linkup-cli/src/remote_local.rs index 90564a1..fe4772b 100644 --- a/linkup-cli/src/remote_local.rs +++ b/linkup-cli/src/remote_local.rs @@ -1,9 +1,8 @@ use url::Url; use crate::{ - background_booting::{load_config, server_config_from_state}, + background_booting::{load_config, ServerConfig}, local_config::{LocalState, ServiceTarget}, - start::{get_state, save_state}, CliError, LINKUP_LOCALSERVER_PORT, }; @@ -13,7 +12,7 @@ pub fn remote(service_names: &[String]) -> Result<(), CliError> { "No service names provided".to_string(), )); } - let mut state = get_state()?; + let mut state = LocalState::load()?; for service_name in service_names { let service = state @@ -24,7 +23,7 @@ pub fn remote(service_names: &[String]) -> Result<(), CliError> { service.current = ServiceTarget::Remote; } - save_state(state.clone())?; + state.save()?; load_server_states(state)?; println!( @@ -42,7 +41,7 @@ pub fn local(service_names: &[String]) -> Result<(), CliError> { )); } - let mut state = get_state()?; + let mut state = LocalState::load()?; for service_name in service_names { let service = state @@ -53,7 +52,7 @@ pub fn local(service_names: &[String]) -> Result<(), CliError> { service.current = ServiceTarget::Local; } - save_state(state.clone())?; + state.save()?; load_server_states(state)?; println!( @@ -68,13 +67,14 @@ fn load_server_states(state: LocalState) -> Result<(), CliError> { let local_url = Url::parse(&format!("http://localhost:{}", LINKUP_LOCALSERVER_PORT)) .expect("linkup url invalid"); - let (local_server_conf, remote_server_conf) = server_config_from_state(&state); + let server_config = ServerConfig::from(&state); + let _ = load_config( &state.linkup.remote, &state.linkup.session_name.clone(), - remote_server_conf, + server_config.remote, )?; - let _ = load_config(&local_url, &state.linkup.session_name, local_server_conf)?; + let _ = load_config(&local_url, &state.linkup.session_name, server_config.local)?; Ok(()) } diff --git a/linkup-cli/src/reset.rs b/linkup-cli/src/reset.rs index 7b63576..46fc39c 100644 --- a/linkup-cli/src/reset.rs +++ b/linkup-cli/src/reset.rs @@ -1,8 +1,8 @@ use crate::{ background_booting::boot_background_services, linkup_file_path, - local_config::{config_path, get_config}, - start::{boot_local_dns, get_state}, + local_config::{config_path, get_config, LocalState}, + start::boot_local_dns, stop::shutdown, CliError, LINKUP_LOCALDNS_INSTALL, }; @@ -10,7 +10,7 @@ use crate::{ // TODO(ostenbom)[2023-09-26]: Config arg shouldn't be needed here, we could use config state for this pub fn reset(config_arg: &Option) -> Result<(), CliError> { // Ensure there is some kind of state from before, otherwise reset doesn't make sense - get_state()?; + LocalState::load()?; shutdown()?; boot_background_services()?; diff --git a/linkup-cli/src/start.rs b/linkup-cli/src/start.rs index 09af151..cbb6cdb 100644 --- a/linkup-cli/src/start.rs +++ b/linkup-cli/src/start.rs @@ -1,5 +1,5 @@ use std::{ - fs::{self, File}, + fs, path::{Path, PathBuf}, }; @@ -10,12 +10,12 @@ use crate::{ linkup_file_path, local_config::{config_to_state, LocalState, YamlLocalConfig}, status::{server_status, ServerStatus}, - CliError, LINKUP_STATE_FILE, + CliError, }; use crate::{services, LINKUP_LOCALDNS_INSTALL}; pub fn start(config_arg: &Option) -> Result<(), CliError> { - let previous_state = get_state(); + let previous_state = LocalState::load(); let config_path = config_path(config_arg)?; let input_config = get_config(&config_path)?; @@ -30,7 +30,7 @@ pub fn start(config_arg: &Option) -> Result<(), CliError> { state.linkup.tunnel = ps.linkup.tunnel; } - save_state(state.clone())?; + state.save()?; // Set env vars to linkup for service in &state.services { @@ -51,42 +51,6 @@ pub fn start(config_arg: &Option) -> Result<(), CliError> { Ok(()) } -pub fn get_state() -> Result { - if let Err(e) = File::open(linkup_file_path(LINKUP_STATE_FILE)) { - return Err(CliError::NoState(e.to_string())); - } - - let content = match fs::read_to_string(linkup_file_path(LINKUP_STATE_FILE)) { - Ok(content) => content, - Err(e) => return Err(CliError::NoState(e.to_string())), - }; - - match serde_yaml::from_str(&content) { - Ok(config) => Ok(config), - Err(e) => Err(CliError::NoState(e.to_string())), - } -} - -pub fn save_state(state: LocalState) -> Result<(), CliError> { - let yaml_string = match serde_yaml::to_string(&state) { - Ok(yaml) => yaml, - Err(_) => { - return Err(CliError::SaveState( - "Failed to serialize the state into YAML".to_string(), - )) - } - }; - - if fs::write(linkup_file_path(LINKUP_STATE_FILE), yaml_string).is_err() { - return Err(CliError::SaveState(format!( - "Failed to write the state file at {}", - linkup_file_path(LINKUP_STATE_FILE).display() - ))); - } - - Ok(()) -} - fn set_service_env(directory: String, config_path: String) -> Result<(), CliError> { let config_dir = Path::new(&config_path).parent().ok_or_else(|| { CliError::SetServiceEnv( @@ -130,7 +94,7 @@ fn set_service_env(directory: String, config_path: String) -> Result<(), CliErro } fn check_local_not_started() -> Result<(), CliError> { - let state = get_state()?; + let state = LocalState::load()?; for service in state.services { if service.local == service.remote { continue; diff --git a/linkup-cli/src/status.rs b/linkup-cli/src/status.rs index ceb5b55..d50f424 100644 --- a/linkup-cli/src/status.rs +++ b/linkup-cli/src/status.rs @@ -5,7 +5,6 @@ use std::{thread, time::Duration}; use crate::{ local_config::{LocalState, ServiceTarget}, - start::get_state, CliError, LINKUP_LOCALSERVER_PORT, }; @@ -57,7 +56,7 @@ impl From> for ServerStatus } pub fn status(json: bool, all: bool) -> Result<(), CliError> { - let state = get_state()?; + let state = LocalState::load()?; let (tx, rx) = std::sync::mpsc::channel(); linkup_status(tx.clone(), &state); diff --git a/linkup-cli/src/stop.rs b/linkup-cli/src/stop.rs index 4ac8d10..77aca4b 100644 --- a/linkup-cli/src/stop.rs +++ b/linkup-cli/src/stop.rs @@ -4,8 +4,8 @@ use std::path::{Path, PathBuf}; use nix::sys::signal::Signal; use crate::env_files::clear_env_file; +use crate::local_config::LocalState; use crate::signal::{get_pid, send_signal, PidError}; -use crate::start::get_state; use crate::{ linkup_file_path, services, CliError, LINKUP_CLOUDFLARED_PID, LINKUP_LOCALDNS_INSTALL, LINKUP_LOCALSERVER_PID_FILE, @@ -13,7 +13,7 @@ use crate::{ pub fn stop() -> Result<(), CliError> { // Reset env vars back to what they were before - let state = get_state()?; + let state = LocalState::load()?; for service in &state.services { let remove_res = match &service.directory { Some(d) => remove_service_env(d.clone(), state.linkup.config_path.clone()), diff --git a/linkup/src/session.rs b/linkup/src/session.rs index f63a363..25a7ee2 100644 --- a/linkup/src/session.rs +++ b/linkup/src/session.rs @@ -64,7 +64,7 @@ pub struct StorableSession { pub cache_routes: Option>, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct StorableService { pub name: String, pub location: Url,