public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 3/5] api: filter snapshot counts
Date: Thu, 12 Nov 2020 11:30:32 +0100	[thread overview]
Message-ID: <20201112103034.159849-4-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20201112103034.159849-1-f.gruenbichler@proxmox.com>

unprivileged users should only see the counts related to their part of
the datastore.

while we're at it, switch to a list groups, filter groups, count
snapshots approach (like list_snapshots) to speedup calls to this
endpoint when many unprivileged users share a datastore.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs | 93 +++++++++++++++++++------------------
 src/api2/types/mod.rs       |  2 +-
 2 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 857fb903..6d9de4e4 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -472,51 +472,40 @@ pub fn list_snapshots (
         })
 }
 
-fn get_snapshots_count(store: &DataStore) -> Result<Counts, Error> {
+fn get_snapshots_count(store: &DataStore, filter_owner: Option<&Authid>) -> Result<Counts, Error> {
     let base_path = store.base_path();
-    let backup_list = BackupInfo::list_backups(&base_path)?;
-    let mut groups = HashSet::new();
-
-    let mut result = Counts {
-        ct: None,
-        host: None,
-        vm: None,
-        other: None,
-    };
-
-    for info in backup_list {
-        let group = info.backup_dir.group();
-
-        let id = group.backup_id();
-        let backup_type = group.backup_type();
-
-        let mut new_id = false;
-
-        if groups.insert(format!("{}-{}", &backup_type, &id)) {
-            new_id = true;
-        }
+    let groups = BackupInfo::list_backup_groups(&base_path)?;
 
-        let mut counts = match backup_type {
-            "ct" => result.ct.take().unwrap_or(Default::default()),
-            "host" => result.host.take().unwrap_or(Default::default()),
-            "vm" => result.vm.take().unwrap_or(Default::default()),
-            _ => result.other.take().unwrap_or(Default::default()),
-        };
+    groups.iter()
+        .filter(|group| {
+            let owner = match store.get_owner(&group) {
+                Ok(owner) => owner,
+                Err(err) => {
+                    eprintln!("Failed to get owner of group '{}' - {}", group, err);
+                    return false;
+                },
+            };
 
-        counts.snapshots += 1;
-        if new_id {
-            counts.groups +=1;
-        }
+            match filter_owner {
+                Some(filter) => check_backup_owner(&owner, filter).is_ok(),
+                None => true,
+            }
+        })
+        .try_fold(Counts::default(), |mut counts, group| {
+            let snapshot_count = group.list_backups(&base_path)?.len() as u64;
+
+            let type_count = match group.backup_type() {
+                "ct" => counts.ct.get_or_insert(Default::default()),
+                "vm" => counts.vm.get_or_insert(Default::default()),
+                "host" => counts.host.get_or_insert(Default::default()),
+                _ => counts.other.get_or_insert(Default::default()),
+            };
 
-        match backup_type {
-            "ct" => result.ct = Some(counts),
-            "host" => result.host = Some(counts),
-            "vm" => result.vm = Some(counts),
-            _ => result.other = Some(counts),
-        }
-    }
+            type_count.groups += 1;
+            type_count.snapshots += snapshot_count;
 
-    Ok(result)
+            Ok(counts)
+        })
 }
 
 #[api(
@@ -546,15 +535,27 @@ pub fn status(
     store: String,
     verbose: bool,
     _info: &ApiMethod,
-    _rpcenv: &mut dyn RpcEnvironment,
+    rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<DataStoreStatus, Error> {
     let datastore = DataStore::lookup_datastore(&store)?;
     let storage = crate::tools::disks::disk_usage(&datastore.base_path())?;
-    let (counts, gc_status) = match verbose {
-        true => {
-            (Some(get_snapshots_count(&datastore)?), Some(datastore.last_gc_status()))
-        },
-        false => (None, None),
+    let (counts, gc_status) = if verbose {
+        let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+        let user_info = CachedUserInfo::new()?;
+
+        let store_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
+        let filter_owner = if store_privs & PRIV_DATASTORE_AUDIT != 0 {
+            None
+        } else {
+            Some(&auth_id)
+        };
+
+        let counts = Some(get_snapshots_count(&datastore, filter_owner)?);
+        let gc_status = Some(datastore.last_gc_status());
+
+        (counts, gc_status)
+    } else {
+        (None, None)
     };
 
     Ok(DataStoreStatus {
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 44cfef94..d7d5a8af 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -692,7 +692,7 @@ pub struct TypeCounts {
         },
     },
 )]
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Default)]
 /// Counts of groups/snapshots per BackupType.
 pub struct Counts {
     /// The counts for CT backups
-- 
2.20.1





  parent reply	other threads:[~2020-11-12 10:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-12 10:30 [pbs-devel] [PATCH proxmox-backup 0/5] improve listing API Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 1/5] improve group/snapshot listing Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 2/5] api: make expensive parts of datastore status opt-in Fabian Grünbichler
2020-11-12 10:30 ` Fabian Grünbichler [this message]
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 4/5] drop now unused BackupInfo::list_backups Fabian Grünbichler
2020-11-12 10:30 ` [pbs-devel] [PATCH proxmox-backup 5/5] api: include store in invalid owner errors Fabian Grünbichler
2020-11-18 10:14 ` [pbs-devel] applied series: [PATCH proxmox-backup 0/5] improve listing API Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201112103034.159849-4-f.gruenbichler@proxmox.com \
    --to=f.gruenbichler@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal