From efac9ea04afaa10ab42fa392356013559ace05f6 Mon Sep 17 00:00:00 2001
From: Joerg Herbel <joerg.herbel@checkmk.com>
Date: Tue, 20 Aug 2024 08:40:51 +0200
Subject: [PATCH] Stop using temporary files when writing sections

On some Linux distributions such Fedora and openSUSE, `$TMPDIR` points
to a tmpfs file system. This is problematic because
`tempfile::NamedTempFile::persists` does not work across file systems.

Originally, we introduced temporary files to avoid race conditions
between the scheduler writing a result and the agent plugin reading the
results. However, in the meantime, we also introduced explicit locking,
so this is not an issue anymore.

Note that we still first attempt to write to `{result_path}.tmp` and
then rename the file. This avoids breaking earlier, valid result files
due to a failure during the write operation. In case the write operation
failes, we attempt to remove  `{result_path}.tmp`. However, leaving an
invalid file behind won't break the agent plugin, since it ignores
invalid files.

CMK-18766
---
 src/section.rs | 67 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 17 deletions(-)

diff --git a/src/section.rs b/src/section.rs
index 5404903e..14cce3a9 100644
--- a/src/section.rs
+++ b/src/section.rs
@@ -1,13 +1,11 @@
 use super::lock::{Locker, LockerError};
 
 use crate::termination::Terminate;
-use anyhow::{Context, Result as AnyhowResult};
-use camino::Utf8Path;
+use anyhow::{anyhow, Context, Result as AnyhowResult};
+use camino::{Utf8Path, Utf8PathBuf};
 use serde::{Deserialize, Serialize};
-use std::fs::read_to_string;
-use std::io::Write;
+use std::fs;
 use std::path::Path;
-use tempfile::NamedTempFile;
 use walkdir::{DirEntry, WalkDir};
 
 #[derive(Deserialize, Serialize, Clone, Debug, PartialEq)]
@@ -73,22 +71,57 @@ pub fn read(directory: impl AsRef<Path>, locker: &Locker) -> Result<Vec<Section>
 }
 
 fn write(section: &Section, path: impl AsRef<Utf8Path>, locker: &Locker) -> Result<(), Terminate> {
-    let path = path.as_ref();
-    let section = serde_json::to_string(&section).unwrap();
-    let mut file = NamedTempFile::new().context("Opening tempfile failed")?;
-    file.write_all(section.as_bytes()).context(format!(
-        "Writing tempfile failed, {}",
-        file.path().display()
-    ))?;
-
     let lock = locker.wait_for_write_lock()?;
-    file.persist(path)
-        .context(format!("Persisting tempfile failed, final_path: {path}"))?;
-    Ok(lock.release()?)
+    let mut errors = write_to_tmp_and_move(
+        &serde_json::to_string(section).unwrap(),
+        path.as_ref(),
+        &Utf8PathBuf::from(format!("{}.tmp", path.as_ref())),
+    );
+    if let Err(err) = lock.release() {
+        errors.push(anyhow!(err));
+    }
+    if errors.is_empty() {
+        return Ok(());
+    }
+    let mut error_message =
+        "Encountered the following errors while attempting to write section:".to_string();
+    for error in errors {
+        error_message = format!("{error_message}\n{error:?}");
+    }
+    Err(Terminate::Unrecoverable(anyhow!(error_message)))
+}
+
+fn write_to_tmp_and_move(
+    content: &str,
+    path: &Utf8Path,
+    tmp_path: &Utf8Path,
+) -> Vec<anyhow::Error> {
+    let mut errors =
+        match fs::write(tmp_path, content).context(format!("Writing to {tmp_path} failed")) {
+            Ok(_) => match fs::rename(tmp_path, path)
+                .context(format!("Renaming {tmp_path} to {path} failed"))
+            {
+                Ok(_) => vec![],
+
+                Err(err) => vec![err],
+            },
+            Err(err) => vec![err],
+        };
+    if errors.is_empty() {
+        return errors;
+    }
+    if tmp_path.exists() {
+        if let Err(err) =
+            fs::remove_file(tmp_path).context(format!("{tmp_path} exists and removing it failed"))
+        {
+            errors.push(err)
+        }
+    }
+    errors
 }
 
 fn read_entry(entry: Result<DirEntry, walkdir::Error>) -> AnyhowResult<Section> {
     let entry = entry?;
-    let raw = read_to_string(entry.path())?;
+    let raw = fs::read_to_string(entry.path())?;
     Ok(serde_json::from_str(&raw)?)
 }