[pbs-devel] [PATCH proxmox-backup v2 4/8] api: admin: run configured sync jobs when a datastore is mounted

[Hannes Laimer]

On 5/30/25 15:08, Christian Ebner wrote:
> On 5/30/25 13:45, Hannes Laimer wrote:
>> added some inline comments, I'll probably send a v3 addressing some of
>> the things you mentioned
>>
>> On 5/30/25 12:02, Christian Ebner wrote:
>>> please see some comments inline
>>>
>>> On 5/15/25 14:41, Hannes Laimer wrote:
>>>> When a datastore is mounted, spawn a new task to run all sync jobs
>>>> marked with `run-on-mount`. These jobs run sequentially and include
>>>> any job for which the mounted datastore is:
>>>>
>>>> - The source or target in a local push/pull job
>>>> - The source in a push job to a remote datastore
>>>> - The target in a pull job from a remote datastore
>>>>
>>>> Signed-off-by: Hannes Laimer <h.laimer at proxmox.com>
>>>> +                let _ = WorkerTask::spawn(
>>>> +                    "mount-sync-jobs",
>>>> +                    Some(store),
>>>> +                    auth_id.to_string(),
>>>> +                    false,
>>>> +                    move |worker| async move 
>>>> { do_sync_jobs(jobs_to_run, worker).await },
>>>> +                );
>>>> +            }
>>>> +            Ok(())
>>>> +        },
>>>
>>> comment: all of above is executed within a api endpoint flagged as 
>>> protected! This however leads to the sync job to be executed by the 
>>> proxmox-backup-proxy, all the synced contents therefore written and 
>>> owner by the root user instead of the backup user.
>>>
>>
>> actually true, good catch! I didn't even check that, I just assumed 
>> we'd explicitly set the owner whenever we create dirs/files during 
>> sync. We do this for garbage collection and in basically all other 
>> places IIRC.
>>
>> So, I think we should also do that for syncs, can you think of a reason
>> to not do that? If not, I'll prepare a patch for that.
> 
> As discussed off-list a bit, IMHO running the whole sync job code paths 
> in the privileged api server is not ideal and would weaken the privilege 
> separation. But I do agree that this is already been done in some places 
> and would be probably easy to approach.
> 
> An option would be to add an (unprivileged) api endpoint which allows to 
> pass a list of sync jobs to be executed sequentially, e.g. `POST /api2/ 
> json/admin/sync/run-jobs`.
> 
> Given that we found no better approach other then the two proposed ones,
> maybe somebody else has an opinion/suggestion on this?

For the sake of completeness, spoke with @Thomas off-list, and I'll use 
the existing run_sync_job endpoint to start the jobs, and the UPID to do
polling on the status. This is in line with what we do in a few places
already and it does keep privileged and non-privileged stuff strictly
separated.