[pbs-devel] [PATCH proxmox-backup 3/3] pull: only remove owned groups
Thomas Lamprecht
t.lamprecht at proxmox.com
Mon Jan 18 06:57:11 CET 2021
On 15.01.21 11:48, Fabian Grünbichler wrote:
> we also only create/add snapshots to owned groups when syncing, so
> removing groups with different ownership is a rather confusing
> side-effect..
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
>
> Notes:
> came up in the forum, the restricted behaviour is better for mixed usage as
> sync target and regular datastore, or sync target for multiple sources with
> different owners..
>
> datastores just used as sync target for a single job should still behave the
> same (they have a single owner), datastores used as sync target for multiple
> jobs with the same owner should still not use remove_vanished.. we'd need to
> keep track of the sync origin inside the group for that to work..
>
> src/client/pull.rs | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/client/pull.rs b/src/client/pull.rs
> index 15514374..33a6c0f1 100644
> --- a/src/client/pull.rs
> +++ b/src/client/pull.rs
> @@ -590,11 +590,15 @@ pub async fn pull_store(
>
> let mut errors = false;
>
> - let mut new_groups = std::collections::HashSet::new();
> + let mut remote_groups = std::collections::HashSet::new();
> for item in list.iter() {
> - new_groups.insert(BackupGroup::new(&item.backup_type, &item.backup_id));
> + remote_groups.insert(BackupGroup::new(&item.backup_type, &item.backup_id));
> }
>
> + let correct_owner = |owner: &Authid, auth_id: &Authid| -> bool {
> + owner == auth_id || (owner.is_token() && &Authid::from(owner.user().clone()) == auth_id)
> + };
> +
> let mut progress = StoreProgress::new(list.len() as u64);
>
> for (done, item) in list.into_iter().enumerate() {
> @@ -617,7 +621,7 @@ pub async fn pull_store(
> };
>
> // permission check
> - if auth_id != owner {
> + if !correct_owner(&owner, &auth_id) {
this is now also changed to include token owned groups, or? As the `correct_owner` closure
checks not only the replaced (negated) auth_id == owner but also an explicit token check?
(did not looked to much at code out of context, just FYI)
> // only the owner is allowed to create additional snapshots
> worker.log(format!(
> "sync group {}/{} failed - owner check failed ({} != {})",
> @@ -645,9 +649,16 @@ pub async fn pull_store(
>
> if delete {
> let result: Result<(), Error> = proxmox::try_block!({
> - let local_groups = BackupInfo::list_backup_groups(&tgt_store.base_path())?;
> - for local_group in local_groups {
> - if new_groups.contains(&local_group) {
> + let local_owned_groups: Vec<BackupGroup> =
> + BackupInfo::list_backup_groups(&tgt_store.base_path())?
> + .into_iter()
> + .filter(|group| match tgt_store.get_owner(&group) {
> + Ok(owner) => correct_owner(&owner, &auth_id),
> + Err(_) => false,
> + })
> + .collect();
> + for local_group in local_owned_groups {
> + if remote_groups.contains(&local_group) {
> continue;
> }
> worker.log(format!(
>
More information about the pbs-devel
mailing list