diff --git a/rust/gui-client/src-common/src/controller.rs b/rust/gui-client/src-common/src/controller.rs index 0fcf7bda1..31ec2e998 100644 --- a/rust/gui-client/src-common/src/controller.rs +++ b/rust/gui-client/src-common/src/controller.rs @@ -386,16 +386,19 @@ impl<'a, I: GuiIntegration> Controller<'a, I> { async fn handle_request(&mut self, req: ControllerRequest) -> Result<(), Error> { match req { Req::ApplySettings(settings) => { - let filter = firezone_logging::try_filter(&self.advanced_settings.log_filter) + let filter = firezone_logging::try_filter(&settings.log_filter) .context("Couldn't parse new log filter directives")?; self.advanced_settings = *settings; + self.log_filter_reloader .reload(filter) .context("Couldn't reload log filter")?; - self.ipc_client.send_msg(&IpcClientMsg::ReloadLogFilter).await?; + self.ipc_client.send_msg(&IpcClientMsg::ApplyLogFilter { directives: self.advanced_settings.log_filter.clone() }).await?; + tracing::debug!( "Applied new settings. Log level will take effect immediately." ); + // Refresh the menu in case the favorites were reset. self.refresh_system_tray_menu()?; } diff --git a/rust/gui-client/src-common/src/settings.rs b/rust/gui-client/src-common/src/settings.rs index 2cbfcff13..924daed28 100644 --- a/rust/gui-client/src-common/src/settings.rs +++ b/rust/gui-client/src-common/src/settings.rs @@ -2,12 +2,10 @@ //! advanced settings and code for manipulating diagnostic logs. use anyhow::{Context as _, Result}; -use atomicwrites::{AtomicFile, OverwriteBehavior}; use connlib_model::ResourceId; use firezone_headless_client::known_dirs; -use firezone_logging::std_dyn_err; use serde::{Deserialize, Serialize}; -use std::{collections::HashSet, io::Write, path::PathBuf}; +use std::{collections::HashSet, path::PathBuf}; use url::Url; #[derive(Clone, Deserialize, Serialize)] @@ -65,21 +63,12 @@ pub async fn save(settings: &AdvancedSettings) -> Result<()> { let dir = path .parent() .context("settings path should have a parent")?; + tokio::fs::create_dir_all(dir).await?; tokio::fs::write(&path, serde_json::to_string(settings)?).await?; - // Don't create the dir for the log filter file, that's the IPC service's job. - // If it isn't there for some reason yet, just log an error and move on. - let log_filter_path = known_dirs::ipc_log_filter().context("`ipc_log_filter` failed")?; - let f = AtomicFile::new(&log_filter_path, OverwriteBehavior::AllowOverwrite); - // Note: Blocking file write in async function - if let Err(error) = f.write(|f| f.write_all(settings.log_filter.as_bytes())) { - tracing::error!( - error = std_dyn_err(&error), - ?log_filter_path, - "Couldn't write log filter file for IPC service" - ); - } + tracing::debug!(?path, "Saved settings"); + Ok(()) } diff --git a/rust/headless-client/src/ipc_service.rs b/rust/headless-client/src/ipc_service.rs index f5de4195f..105b0ddd0 100644 --- a/rust/headless-client/src/ipc_service.rs +++ b/rust/headless-client/src/ipc_service.rs @@ -3,13 +3,14 @@ use crate::{ ConnlibMsg, LogFilterReloader, }; use anyhow::{bail, Context as _, Result}; +use atomicwrites::{AtomicFile, OverwriteBehavior}; use clap::Parser; use connlib_model::ResourceView; use firezone_bin_shared::{ platform::{tcp_socket_factory, udp_socket_factory, DnsControlMethod}, TunDeviceManager, TOKEN_ENV_KEY, }; -use firezone_logging::{anyhow_dyn_err, sentry_layer, telemetry_span}; +use firezone_logging::{anyhow_dyn_err, sentry_layer, std_dyn_err, telemetry_span}; use firezone_telemetry::Telemetry; use futures::{ future::poll_fn, @@ -20,9 +21,15 @@ use phoenix_channel::LoginUrl; use secrecy::SecretString; use serde::{Deserialize, Serialize}; use std::{ - collections::BTreeSet, io, net::IpAddr, path::PathBuf, pin::pin, sync::Arc, time::Duration, + collections::BTreeSet, + io::{self, Write}, + net::IpAddr, + path::PathBuf, + pin::pin, + sync::Arc, + time::Duration, }; -use tokio::{sync::mpsc, task::spawn_blocking, time::Instant}; +use tokio::{sync::mpsc, time::Instant}; use tracing_subscriber::{layer::SubscriberExt, reload, EnvFilter, Layer, Registry}; use url::Url; @@ -83,7 +90,9 @@ pub enum ClientMsg { token: String, }, Disconnect, - ReloadLogFilter, + ApplyLogFilter { + directives: String, + }, Reset, SetDns(Vec), SetDisabledResources(BTreeSet), @@ -498,9 +507,16 @@ impl<'a> Handler<'a> { .await .context("Failed to send `DisconnectedGracefully`")?; } - ClientMsg::ReloadLogFilter => { - let filter = spawn_blocking(get_log_filter).await??; - self.log_filter_reloader.reload(filter)?; + ClientMsg::ApplyLogFilter { directives } => { + self.log_filter_reloader.reload(directives.clone())?; + + let path = known_dirs::ipc_log_filter()?; + + if let Err(e) = AtomicFile::new(&path, OverwriteBehavior::AllowOverwrite) + .write(|f| f.write_all(directives.as_bytes())) + { + tracing::warn!(path = %path.display(), %directives, error = std_dyn_err(&e), "Failed to write new log directives"); + } } ClientMsg::Reset => { if self.last_connlib_start_instant.is_some() { @@ -673,11 +689,8 @@ pub(crate) fn get_log_filter() -> Result { return Ok(filter); } - if let Ok(filter) = std::fs::read_to_string( - known_dirs::ipc_log_filter() - .context("Failed to compute directory for log filter config file")?, - ) - .map(|s| s.trim().to_string()) + if let Ok(filter) = + std::fs::read_to_string(known_dirs::ipc_log_filter()?).map(|s| s.trim().to_string()) { return Ok(filter); } diff --git a/rust/headless-client/src/known_dirs.rs b/rust/headless-client/src/known_dirs.rs index a01676570..d10751c0d 100644 --- a/rust/headless-client/src/known_dirs.rs +++ b/rust/headless-client/src/known_dirs.rs @@ -7,9 +7,11 @@ //! //! I wanted the ProgramData folder on Windows, which `dirs` alone doesn't provide. -pub use platform::{ipc_service_config, ipc_service_logs, logs, runtime, session, settings}; +use anyhow::{Context as _, Result}; use std::path::PathBuf; +pub use platform::{ipc_service_config, ipc_service_logs, logs, runtime, session, settings}; + #[cfg(target_os = "linux")] #[path = "known_dirs/linux.rs"] pub mod platform; @@ -22,8 +24,10 @@ pub mod platform; #[path = "known_dirs/windows.rs"] pub mod platform; -pub fn ipc_log_filter() -> Option { - Some(ipc_service_config()?.join("log-filter")) +pub fn ipc_log_filter() -> Result { + Ok(ipc_service_config() + .context("Failed to compute `ipc_service_config` directory")? + .join("log-filter")) } #[cfg(test)] diff --git a/rust/headless-client/src/known_dirs/linux.rs b/rust/headless-client/src/known_dirs/linux.rs index 546b34d53..c2a2dd88b 100644 --- a/rust/headless-client/src/known_dirs/linux.rs +++ b/rust/headless-client/src/known_dirs/linux.rs @@ -1,11 +1,9 @@ use firezone_bin_shared::BUNDLE_ID; use std::path::PathBuf; -/// Path for IPC service config that either the IPC service or GUI can write +/// Path for IPC service config that the IPC service can write /// -/// e.g. the device ID should only be written by the IPC service, and -/// the log filter should only be written by the GUI. No file should be written -/// by both programs. All writes should use `atomicwrites`. +/// All writes should use `atomicwrites`. /// /// On Linux, `/var/lib/$BUNDLE_ID/config/firezone-id` /// diff --git a/rust/headless-client/src/known_dirs/windows.rs b/rust/headless-client/src/known_dirs/windows.rs index 1154ab76b..a82592c3c 100644 --- a/rust/headless-client/src/known_dirs/windows.rs +++ b/rust/headless-client/src/known_dirs/windows.rs @@ -2,11 +2,9 @@ use firezone_bin_shared::{platform::app_local_data_dir, BUNDLE_ID}; use known_folders::{get_known_folder_path, KnownFolder}; use std::path::PathBuf; -/// Path for IPC service config that either the IPC service or GUI can write +/// Path for IPC service config that the IPC service can write /// -/// e.g. the device ID should only be written by the IPC service, and -/// the log filter should only be written by the GUI. No file should be written -/// by both programs. All writes should use `atomicwrites`. +/// All writes should use `atomicwrites`. /// /// On Windows, `C:/ProgramData/$BUNDLE_ID/config` pub fn ipc_service_config() -> Option {