[pbs-devel] [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[pbs-devel] [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Matthias Heiserer]

It pulls to root namespace instead.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 src/bin/proxmox-backup-manager.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 9857d701..58e7e33a 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -305,7 +305,7 @@ async fn pull_datastore(
     }
 
     if ns.is_some() {
-        args["local-ns"] = json!(ns);
+        args["ns"] = json!(ns);
     }
 
     if max_depth.is_some() {
-- 
2.30.2

[pbs-devel] applied: [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Fabian Grünbichler]

thanks for the quick fix!

On September 20, 2022 12:08 pm, Matthias Heiserer wrote:
> It pulls to root namespace instead.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  src/bin/proxmox-backup-manager.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
> index 9857d701..58e7e33a 100644
> --- a/src/bin/proxmox-backup-manager.rs
> +++ b/src/bin/proxmox-backup-manager.rs
> @@ -305,7 +305,7 @@ async fn pull_datastore(
>      }
>  
>      if ns.is_some() {
> -        args["local-ns"] = json!(ns);
> +        args["ns"] = json!(ns);
>      }
>  
>      if max_depth.is_some() {
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 

Re: [pbs-devel] applied: [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Thomas Lamprecht]

Am 20/09/2022 um 12:19 schrieb Fabian Grünbichler:
> thanks for the quick fix!

You were quicker (I wanted to amend the commit message a bit) and currently
wondering why that POST call went through anyway, as the extra unknown
parameter should cause a higher level error?

Re: [pbs-devel] applied: [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Fabian Grünbichler]

On September 20, 2022 12:24 pm, Thomas Lamprecht wrote:
> Am 20/09/2022 um 12:19 schrieb Fabian Grünbichler:
>> thanks for the quick fix!
> 
> You were quicker (I wanted to amend the commit message a bit) and currently
> wondering why that POST call went through anyway, as the extra unknown
> parameter should cause a higher level error?

I was looking in parallel based on the forum report and then saw the 
patch coming in ;)

the API defaults to (silently) allowing additional properties, which is 
why this is neither an error nor logged anywhere. this behaviour comes 
from PVE IIRC.

Re: [pbs-devel] applied: [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Thomas Lamprecht]

Am 20/09/2022 um 12:43 schrieb Fabian Grünbichler:
> On September 20, 2022 12:24 pm, Thomas Lamprecht wrote:
>> Am 20/09/2022 um 12:19 schrieb Fabian Grünbichler:
>>> thanks for the quick fix!
>>
>> You were quicker (I wanted to amend the commit message a bit) and currently
>> wondering why that POST call went through anyway, as the extra unknown
>> parameter should cause a higher level error?
> 
> I was looking in parallel based on the forum report and then saw the 
> patch coming in ;)
> 
> the API defaults to (silently) allowing additional properties, which is 
> why this is neither an error nor logged anywhere. this behaviour comes 
> from PVE IIRC.

That normally should require a explicit `additional_properties: true,` which
we do not set here. The ObjectSchema has it set to false by default and PVE's
default is also false IIRC, resulting in a 400 error..

Re: [pbs-devel] applied: [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Fabian Grünbichler]

On September 20, 2022 12:48 pm, Thomas Lamprecht wrote:
> Am 20/09/2022 um 12:43 schrieb Fabian Grünbichler:
>> On September 20, 2022 12:24 pm, Thomas Lamprecht wrote:
>>> Am 20/09/2022 um 12:19 schrieb Fabian Grünbichler:
>>>> thanks for the quick fix!
>>>
>>> You were quicker (I wanted to amend the commit message a bit) and currently
>>> wondering why that POST call went through anyway, as the extra unknown
>>> parameter should cause a higher level error?
>> 
>> I was looking in parallel based on the forum report and then saw the 
>> patch coming in ;)
>> 
>> the API defaults to (silently) allowing additional properties, which is 
>> why this is neither an error nor logged anywhere. this behaviour comes 
>> from PVE IIRC.
> 
> That normally should require a explicit `additional_properties: true,` which
> we do not set here. The ObjectSchema has it set to false by default and PVE's
> default is also false IIRC, resulting in a 400 error..
> 

parameters here is an AllOfSchema, and that unconditionally allows 
additional properties.

Re: [pbs-devel] applied: [PATCH proxmox-backup] pbs-manager: fix pull not respecting local ns setting

[Thomas Lamprecht]

Am 20/09/2022 um 13:18 schrieb Fabian Grünbichler:
> On September 20, 2022 12:48 pm, Thomas Lamprecht wrote:
>> Am 20/09/2022 um 12:43 schrieb Fabian Grünbichler:
>>> On September 20, 2022 12:24 pm, Thomas Lamprecht wrote:
>>>> Am 20/09/2022 um 12:19 schrieb Fabian Grünbichler:
>>>>> thanks for the quick fix!
>>>>
>>>> You were quicker (I wanted to amend the commit message a bit) and currently
>>>> wondering why that POST call went through anyway, as the extra unknown
>>>> parameter should cause a higher level error?
>>>
>>> I was looking in parallel based on the forum report and then saw the 
>>> patch coming in ;)
>>>
>>> the API defaults to (silently) allowing additional properties, which is 
>>> why this is neither an error nor logged anywhere. this behaviour comes 
>>> from PVE IIRC.
>>
>> That normally should require a explicit `additional_properties: true,` which
>> we do not set here. The ObjectSchema has it set to false by default and PVE's
>> default is also false IIRC, resulting in a 400 error..
>>
> 
> parameters here is an AllOfSchema, and that unconditionally allows 
> additional properties.

yuck, that is ugly