[pbs-devel] [PATCH v3 proxmox-backup 2/2] fix: #4761: introduce overwrite bitflags for fine grained overwrites

[Christian Ebner <c.ebner@proxmox.com>]

Adds OverwriteFlags for granular control of which entry types should
overwrite entries present on the filesystem during a restore.

The original overwrite flag is refactored in order to cover all of the
other cases.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since v1:
* rebased to current master

changes since v2:
* use `all()` and `empty()` instead of defining ALL and NONE variants
  for bitflags.
* Derive default instead of explicit impl.
* Fix formating issues by running rustfmt 

 pbs-client/src/catalog_shell.rs   |  9 ++++--
 pbs-client/src/pxar/extract.rs    | 47 ++++++++++++++++++++++---------
 pbs-client/src/pxar/mod.rs        |  2 +-
 proxmox-backup-client/src/main.rs | 34 +++++++++++++++++++++-
 pxar-bin/src/main.rs              | 32 +++++++++++++++++++--
 5 files changed, 105 insertions(+), 19 deletions(-)

diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
index 98af5699..b8aaf8cb 100644
--- a/pbs-client/src/catalog_shell.rs
+++ b/pbs-client/src/catalog_shell.rs
@@ -987,8 +987,13 @@ impl Shell {
             .metadata()
             .clone();
 
-        let extractor =
-            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, false, Flags::DEFAULT);
+        let extractor = crate::pxar::extract::Extractor::new(
+            rootdir,
+            root_meta,
+            true,
+            crate::pxar::extract::OverwriteFlags::empty(),
+            Flags::DEFAULT,
+        );
 
         let mut extractor = ExtractorState::new(
             &mut self.catalog,
diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index 9a95faa0..37399f17 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -9,6 +9,7 @@ use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
 use anyhow::{bail, format_err, Context, Error};
+use bitflags::bitflags;
 use nix::dir::Dir;
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
@@ -33,10 +34,22 @@ pub struct PxarExtractOptions<'a> {
     pub match_list: &'a [MatchEntry],
     pub extract_match_default: bool,
     pub allow_existing_dirs: bool,
-    pub overwrite: bool,
+    pub overwrite_flags: OverwriteFlags,
     pub on_error: Option<ErrorHandler>,
 }
 
+bitflags! {
+    #[derive(Default)]
+    pub struct OverwriteFlags: u8 {
+        /// Overwrite existing entries file content
+        const FILE = 0x1;
+        /// Overwrite existing entry with symlink
+        const SYMLINK = 0x2;
+        /// Overwrite existing entry with hardlink
+        const HARDLINK = 0x4;
+    }
+}
+
 pub type ErrorHandler = Box<dyn FnMut(Error) -> Result<(), Error> + Send>;
 
 pub fn extract_archive<T, F>(
@@ -141,7 +154,7 @@ where
             dir,
             root.metadata().clone(),
             options.allow_existing_dirs,
-            options.overwrite,
+            options.overwrite_flags,
             feature_flags,
         );
 
@@ -345,7 +358,9 @@ where
                         metadata,
                         *size,
                         &mut contents,
-                        self.extractor.overwrite,
+                        self.extractor
+                            .overwrite_flags
+                            .contains(OverwriteFlags::FILE),
                     )
                 } else {
                     Err(format_err!(
@@ -438,7 +453,7 @@ impl std::fmt::Display for PxarExtractContext {
 pub struct Extractor {
     feature_flags: Flags,
     allow_existing_dirs: bool,
-    overwrite: bool,
+    overwrite_flags: OverwriteFlags,
     dir_stack: PxarDirStack,
 
     /// For better error output we need to track the current path in the Extractor state.
@@ -455,13 +470,13 @@ impl Extractor {
         root_dir: Dir,
         metadata: Metadata,
         allow_existing_dirs: bool,
-        overwrite: bool,
+        overwrite_flags: OverwriteFlags,
         feature_flags: Flags,
     ) -> Self {
         Self {
             dir_stack: PxarDirStack::new(root_dir, metadata),
             allow_existing_dirs,
-            overwrite,
+            overwrite_flags,
             feature_flags,
             current_path: Arc::new(Mutex::new(OsString::new())),
             on_error: Box::new(Err),
@@ -551,7 +566,7 @@ impl Extractor {
         match nix::unistd::symlinkat(link, Some(parent), file_name) {
             Ok(()) => {}
             Err(err @ nix::errno::Errno::EEXIST) => {
-                if !self.overwrite {
+                if !self.overwrite_flags.contains(OverwriteFlags::SYMLINK) {
                     return Err(err.into());
                 }
                 // Never unlink directories
@@ -559,7 +574,7 @@ impl Extractor {
                 nix::unistd::unlinkat(Some(parent), file_name, flag)?;
                 nix::unistd::symlinkat(link, Some(parent), file_name)?;
             }
-            Err(err) => return Err(err.into())
+            Err(err) => return Err(err.into()),
         }
 
         metadata::apply_at(
@@ -591,7 +606,7 @@ impl Extractor {
         match dolink() {
             Ok(()) => {}
             Err(err @ nix::errno::Errno::EEXIST) => {
-                if !self.overwrite {
+                if !self.overwrite_flags.contains(OverwriteFlags::HARDLINK) {
                     return Err(err.into());
                 }
                 // Never unlink directories
@@ -599,7 +614,7 @@ impl Extractor {
                 nix::unistd::unlinkat(Some(parent), file_name, flag)?;
                 dolink()?;
             }
-            Err(err) => return Err(err.into())
+            Err(err) => return Err(err.into()),
         }
 
         Ok(())
@@ -1062,7 +1077,13 @@ where
     )
     .with_context(|| format!("unable to open target directory {:?}", destination.as_ref()))?;
 
-    Ok(Extractor::new(dir, metadata, false, false, Flags::DEFAULT))
+    Ok(Extractor::new(
+        dir,
+        metadata,
+        false,
+        OverwriteFlags::empty(),
+        Flags::DEFAULT,
+    ))
 }
 
 pub async fn extract_sub_dir<T, DEST, PATH>(
@@ -1196,7 +1217,7 @@ where
                         .contents()
                         .await
                         .context("found regular file entry without contents in archive")?,
-                    extractor.overwrite,
+                    extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                 )
                 .await?
         }
@@ -1244,7 +1265,7 @@ where
                             &mut decoder
                                 .contents()
                                 .context("found regular file entry without contents in archive")?,
-                            extractor.overwrite,
+                            extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                         )
                         .await?
                 }
diff --git a/pbs-client/src/pxar/mod.rs b/pbs-client/src/pxar/mod.rs
index b042717d..14674b9b 100644
--- a/pbs-client/src/pxar/mod.rs
+++ b/pbs-client/src/pxar/mod.rs
@@ -59,7 +59,7 @@ pub use flags::Flags;
 pub use create::{create_archive, PxarCreateOptions};
 pub use extract::{
     create_tar, create_zip, extract_archive, extract_sub_dir, extract_sub_dir_seq, ErrorHandler,
-    PxarExtractContext, PxarExtractOptions,
+    OverwriteFlags, PxarExtractContext, PxarExtractOptions,
 };
 
 /// The format requires to build sorted directory lookup tables in
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index d9e7b899..1a13291a 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1234,6 +1234,21 @@ We do not extract '.pxar' archives when writing to standard output.
                 optional: true,
                 default: false,
             },
+            "overwrite-files": {
+                description: "overwrite already existing files",
+                optional: true,
+                default: false,
+            },
+            "overwrite-symlinks": {
+                description: "overwrite already existing entries by archives symlink",
+                optional: true,
+                default: false,
+            },
+            "overwrite-hardlinks": {
+                description: "overwrite already existing entries by archives hardlink",
+                optional: true,
+                default: false,
+            },
             "ignore-extract-device-errors": {
                 type: Boolean,
                 description: "ignore errors that occur during device node extraction",
@@ -1252,6 +1267,9 @@ async fn restore(
     ignore_ownership: bool,
     ignore_permissions: bool,
     overwrite: bool,
+    overwrite_files: bool,
+    overwrite_symlinks: bool,
+    overwrite_hardlinks: bool,
     ignore_extract_device_errors: bool,
 ) -> Result<Value, Error> {
     let repo = extract_repository_from_value(&param)?;
@@ -1388,11 +1406,25 @@ async fn restore(
             None
         };
 
+        let mut overwrite_flags = pbs_client::pxar::OverwriteFlags::empty();
+        overwrite_flags.set(pbs_client::pxar::OverwriteFlags::FILE, overwrite_files);
+        overwrite_flags.set(
+            pbs_client::pxar::OverwriteFlags::SYMLINK,
+            overwrite_symlinks,
+        );
+        overwrite_flags.set(
+            pbs_client::pxar::OverwriteFlags::HARDLINK,
+            overwrite_hardlinks,
+        );
+        if overwrite {
+            overwrite_flags.insert(pbs_client::pxar::OverwriteFlags::all());
+        }
+
         let options = pbs_client::pxar::PxarExtractOptions {
             match_list: &[],
             extract_match_default: true,
             allow_existing_dirs,
-            overwrite,
+            overwrite_flags,
             on_error,
         };
 
diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs
index 90887321..bc044035 100644
--- a/pxar-bin/src/main.rs
+++ b/pxar-bin/src/main.rs
@@ -12,7 +12,9 @@ use futures::select;
 use tokio::signal::unix::{signal, SignalKind};
 
 use pathpatterns::{MatchEntry, MatchType, PatternFlag};
-use pbs_client::pxar::{format_single_line_entry, Flags, PxarExtractOptions, ENCODER_MAX_ENTRIES};
+use pbs_client::pxar::{
+    format_single_line_entry, Flags, OverwriteFlags, PxarExtractOptions, ENCODER_MAX_ENTRIES,
+};
 
 use proxmox_router::cli::*;
 use proxmox_schema::api;
@@ -74,10 +76,25 @@ fn extract_archive_from_reader<R: std::io::Read>(
                 default: false,
             },
             "overwrite": {
+                description: "overwrite already existing files, symlinks and hardlinks",
+                optional: true,
+                default: false,
+            },
+            "overwrite-files": {
                 description: "overwrite already existing files",
                 optional: true,
                 default: false,
             },
+            "overwrite-symlinks": {
+                description: "overwrite already existing entries by archives symlink",
+                optional: true,
+                default: false,
+            },
+            "overwrite-hardlinks": {
+                description: "overwrite already existing entries by archives hardlink",
+                optional: true,
+                default: false,
+            },
             "files-from": {
                 description: "File containing match pattern for files to restore.",
                 optional: true,
@@ -116,6 +133,9 @@ fn extract_archive(
     no_acls: bool,
     allow_existing_dirs: bool,
     overwrite: bool,
+    overwrite_files: bool,
+    overwrite_symlinks: bool,
+    overwrite_hardlinks: bool,
     files_from: Option<String>,
     no_device_nodes: bool,
     no_fifos: bool,
@@ -142,6 +162,14 @@ fn extract_archive(
         feature_flags.remove(Flags::WITH_SOCKETS);
     }
 
+    let mut overwrite_flags = OverwriteFlags::empty();
+    overwrite_flags.set(OverwriteFlags::FILE, overwrite_files);
+    overwrite_flags.set(OverwriteFlags::SYMLINK, overwrite_symlinks);
+    overwrite_flags.set(OverwriteFlags::HARDLINK, overwrite_hardlinks);
+    if overwrite {
+        overwrite_flags.insert(OverwriteFlags::all());
+    }
+
     let pattern = pattern.unwrap_or_default();
     let target = target.as_ref().map_or_else(|| ".", String::as_str);
 
@@ -183,7 +211,7 @@ fn extract_archive(
     let options = PxarExtractOptions {
         match_list: &match_list,
         allow_existing_dirs,
-        overwrite,
+        overwrite_flags,
         extract_match_default,
         on_error,
     };
-- 
2.39.2