From 2f235ebd42527edc8605dbc7d5ba352483083771 Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Fri, 3 May 2024 14:13:45 -0500 Subject: [PATCH] chore(gui-client/linux): export all logs, not just app logs (#4830) 17ac1ebe79 looks good on both Linux and Windows ```[tasklist] ### Before merging - [x] Allow GUI to delete IPC service logs - [x] Test Linux - [x] Test Windows ``` --- .../deb_files/firezone-client-ipc.service | 2 + rust/gui-client/src-tauri/src/client/gui.rs | 23 ++-- .../src-tauri/src/client/logging.rs | 130 ++++++++++++++---- rust/gui-client/src/settings.ts | 2 +- 4 files changed, 112 insertions(+), 45 deletions(-) diff --git a/rust/gui-client/src-tauri/deb_files/firezone-client-ipc.service b/rust/gui-client/src-tauri/deb_files/firezone-client-ipc.service index 7238f7055..6cdec430a 100644 --- a/rust/gui-client/src-tauri/deb_files/firezone-client-ipc.service +++ b/rust/gui-client/src-tauri/deb_files/firezone-client-ipc.service @@ -7,6 +7,8 @@ CapabilityBoundingSet=CAP_CHOWN CAP_NET_ADMIN DeviceAllow=/dev/net/tun LockPersonality=true LogsDirectory=dev.firezone.client +# Allow users in `firezone` group to delete log files +LogsDirectoryMode=775 MemoryDenyWriteExecute=true NoNewPrivileges=true PrivateMounts=true diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index f4e3d4b78..ada1e22a3 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -65,20 +65,6 @@ pub(crate) struct Managed { pub inject_faults: bool, } -impl Managed { - #[cfg(debug_assertions)] - /// In debug mode, if `--inject-faults` is passed, sleep for `millis` milliseconds - pub async fn fault_msleep(&self, millis: u64) { - if self.inject_faults { - tokio::time::sleep(std::time::Duration::from_millis(millis)).await; - } - } - - #[cfg(not(debug_assertions))] - /// Does nothing in release mode - pub async fn fault_msleep(&self, _millis: u64) {} -} - // TODO: Replace with `anyhow` gradually per #[allow(dead_code)] #[derive(Debug, thiserror::Error)] @@ -373,6 +359,10 @@ async fn smoke_test(ctlr_tx: CtlrTx) -> Result<()> { }) .await .context("Failed to send ExportLogs request")?; + ctlr_tx + .send(ControllerRequest::ClearLogs) + .await + .context("Failed to send ClearLogs request")?; // Give the app some time to export the zip and reach steady state tokio::time::sleep_until(quit_time).await; @@ -467,6 +457,8 @@ fn handle_system_tray_event(app: &tauri::AppHandle, event: TrayMenuEvent) -> Res pub(crate) enum ControllerRequest { /// The GUI wants us to use these settings in-memory, they've already been saved to disk ApplySettings(AdvancedSettings), + /// Only used for smoke tests + ClearLogs, Disconnected, /// The same as the arguments to `client::logging::export_logs_to` ExportLogs { @@ -588,6 +580,9 @@ impl Controller { "Applied new settings. Log level will take effect at next app start." ); } + Req::ClearLogs => logging::clear_logs_inner() + .await + .context("Failed to clear logs")?, Req::Disconnected => { tracing::info!("Disconnected by connlib"); self.sign_out().await?; diff --git a/rust/gui-client/src-tauri/src/client/logging.rs b/rust/gui-client/src-tauri/src/client/logging.rs index 822cd3a8f..49ff5a5e7 100644 --- a/rust/gui-client/src-tauri/src/client/logging.rs +++ b/rust/gui-client/src-tauri/src/client/logging.rs @@ -7,7 +7,12 @@ use crate::client::{ use anyhow::{bail, Context, Result}; use connlib_client_shared::file_logger; use serde::Serialize; -use std::{fs, io, path::PathBuf, result::Result as StdResult, str::FromStr}; +use std::{ + fs, io, + path::{Path, PathBuf}, + result::Result as StdResult, + str::FromStr, +}; use tokio::task::spawn_blocking; use tracing::subscriber::set_global_default; use tracing_log::LogTracer; @@ -21,10 +26,19 @@ pub(crate) struct Handles { pub _reloader: reload::Handle, } +struct LogPath { + /// Where to find the logs on disk + /// + /// e.g. `/var/log/dev.firezone.client` + src: PathBuf, + /// Where to store the logs in the zip + /// + /// e.g. `connlib` + dst: PathBuf, +} + #[derive(Debug, thiserror::Error)] pub(crate) enum Error { - #[error("Couldn't compute our local AppData path")] - CantFindLocalAppDataFolder, #[error("Couldn't create logs dir: {0}")] CreateDirAll(std::io::Error), #[error("Log filter couldn't be parsed")] @@ -36,8 +50,8 @@ pub(crate) enum Error { } /// Set up logs after the process has started -pub(crate) fn setup(log_filter: &str) -> Result { - let log_path = log_path()?; +pub(crate) fn setup(log_filter: &str) -> Result { + let log_path = app_log_path()?.src; std::fs::create_dir_all(&log_path).map_err(Error::CreateDirAll)?; let (layer, logger) = file_logger::layer(&log_path); @@ -73,8 +87,8 @@ pub(crate) fn debug_command_setup() -> Result<(), Error> { } #[tauri::command] -pub(crate) async fn clear_logs(managed: tauri::State<'_, Managed>) -> StdResult<(), String> { - clear_logs_inner(&managed).await.map_err(|e| e.to_string()) +pub(crate) async fn clear_logs() -> StdResult<(), String> { + clear_logs_inner().await.map_err(|e| e.to_string()) } #[tauri::command] @@ -97,13 +111,14 @@ pub(crate) async fn count_logs() -> StdResult { /// /// This includes the current log file, so we won't write any more logs to disk /// until the file rolls over or the app restarts. -pub(crate) async fn clear_logs_inner(managed: &Managed) -> Result<()> { - let mut dir = tokio::fs::read_dir(log_path()?).await?; - while let Some(entry) = dir.next_entry().await? { - tokio::fs::remove_file(entry.path()).await?; +pub(crate) async fn clear_logs_inner() -> Result<()> { + for log_path in log_paths()?.into_iter().map(|x| x.src) { + let mut dir = tokio::fs::read_dir(log_path).await?; + while let Some(entry) = dir.next_entry().await? { + tokio::fs::remove_file(entry.path()).await?; + } } - managed.fault_msleep(5000).await; Ok(()) } @@ -111,7 +126,7 @@ pub(crate) async fn clear_logs_inner(managed: &Managed) -> Result<()> { pub(crate) fn export_logs_inner(ctlr_tx: CtlrTx) -> Result<()> { let now = chrono::Local::now(); let datetime_string = now.format("%Y_%m_%d-%H-%M"); - let stem = PathBuf::from(format!("connlib-{datetime_string}")); + let stem = PathBuf::from(format!("firezone_logs_{datetime_string}")); let filename = stem.with_extension("zip"); let Some(filename) = filename.to_str() else { bail!("zip filename isn't valid Unicode"); @@ -145,30 +160,58 @@ pub(crate) async fn export_logs_to(path: PathBuf, stem: PathBuf) -> Result<()> { spawn_blocking(move || { let f = fs::File::create(path).context("Failed to create zip file")?; let mut zip = zip::ZipWriter::new(f); - // All files will have the same modified time. Doing otherwise seems to be difficult - let options = zip::write::FileOptions::default(); - let log_path = log_path().context("Failed to compute log dir path")?; - for entry in fs::read_dir(log_path).context("Failed to `read_dir` log dir")? { - let entry = entry.context("Got bad entry from `read_dir`")?; - let Some(path) = stem.join(entry.file_name()).to_str().map(|x| x.to_owned()) else { - bail!("log filename isn't valid Unicode") - }; - zip.start_file(path, options) - .context("`ZipWriter::start_file` failed")?; - let mut f = fs::File::open(entry.path()).context("Failed to open log file")?; - io::copy(&mut f, &mut zip).context("Failed to copy log file into zip")?; + for log_path in log_paths().context("Can't compute log paths")? { + add_dir_to_zip(&mut zip, &log_path.src, &stem.join(log_path.dst))?; } zip.finish().context("Failed to finish zip file")?; - Ok(()) + Ok::<_, anyhow::Error>(()) }) .await .context("Failed to join zip export task")??; Ok(()) } +/// Reads all files in a directory and adds them to a zip file +/// +/// Does not recurse. +/// All files will have the same modified time. Doing otherwise seems to be difficult +fn add_dir_to_zip( + zip: &mut zip::ZipWriter, + src_dir: &Path, + dst_stem: &Path, +) -> Result<()> { + let options = zip::write::FileOptions::default(); + for entry in fs::read_dir(src_dir).context("Failed to `read_dir` log dir")? { + let entry = entry.context("Got bad entry from `read_dir`")?; + let Some(path) = dst_stem + .join(entry.file_name()) + .to_str() + .map(|x| x.to_owned()) + else { + bail!("log filename isn't valid Unicode") + }; + zip.start_file(path, options) + .context("`ZipWriter::start_file` failed")?; + let mut f = fs::File::open(entry.path()).context("Failed to open log file")?; + io::copy(&mut f, zip).context("Failed to copy log file into zip")?; + } + Ok(()) +} + /// Count log files and their sizes pub(crate) async fn count_logs_inner() -> Result { - let mut dir = tokio::fs::read_dir(log_path()?).await?; + // I spent about 5 minutes on this and couldn't get it to work with `Stream` + let mut total_count = FileCount::default(); + for log_path in log_paths()? { + let count = count_one_dir(&log_path.src).await?; + total_count.files += count.files; + total_count.bytes += count.bytes; + } + Ok(total_count) +} + +async fn count_one_dir(path: &Path) -> Result { + let mut dir = tokio::fs::read_dir(path).await?; let mut file_count = FileCount::default(); while let Some(entry) = dir.next_entry().await? { @@ -180,7 +223,34 @@ pub(crate) async fn count_logs_inner() -> Result { Ok(file_count) } -/// Wrapper around `known_dirs::logs` -pub(crate) fn log_path() -> Result { - known_dirs::logs().ok_or(Error::CantFindLocalAppDataFolder) +#[cfg(target_os = "linux")] +fn log_paths() -> Result> { + Ok(vec![ + LogPath { + // TODO: This is magic, it must match the systemd file + src: PathBuf::from("/var/log").join(connlib_shared::BUNDLE_ID), + dst: PathBuf::from("connlib"), + }, + app_log_path()?, + ]) +} + +/// Windows doesn't have separate connlib logs until #3712 merges +#[cfg(not(target_os = "linux"))] +fn log_paths() -> Result> { + Ok(vec![app_log_path()?]) +} + +/// Log dir for just the GUI app +/// +/// e.g. `$HOME/.cache/dev.firezone.client/data/logs` +/// or `%LOCALAPPDATA%/dev.firezone.client/data/logs` +/// +/// On Windows this also happens to contain the connlib logs, +/// until #3712 merges +fn app_log_path() -> Result { + Ok(LogPath { + src: known_dirs::logs().context("Can't compute app log dir")?, + dst: PathBuf::from("app"), + }) } diff --git a/rust/gui-client/src/settings.ts b/rust/gui-client/src/settings.ts index 0671020b8..1c7f95762 100644 --- a/rust/gui-client/src/settings.ts +++ b/rust/gui-client/src/settings.ts @@ -154,7 +154,7 @@ async function clearLogs() { console.error(e); }) .finally(() => { - logCountOutput.innerText = "0 files, 0 MB"; + countLogs(); unlockLogsForm(); }); }