[pve-devel] [PATCH access-control 1/4] domain sync: make options actually required

[Thomas Lamprecht]

On 4/6/20 1:31 PM, Dominik Csapak wrote:
> we want the api options to be optional, but only as long as there are
> default values set in the realm config
> 
> since they are all marked as optional (else they would be required in
> the api) this check did not work as intended
> 
> instead, check if there exists a 'default' instead (which we then use)
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/API2/Domains.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm
> index 8ae1db0..b42d4f6 100644
> --- a/PVE/API2/Domains.pm
> +++ b/PVE/API2/Domains.pm
> @@ -359,7 +359,7 @@ my $parse_sync_opts = sub {
>  	} elsif (!exists $res->{$opt}) {
>  	    raise_param_exc({
>  		"$opt" => 'Not passed as parameter and not defined in realm default sync options.'
> -	    }) if !$fmt->{optional};
> +	    }) if !exists $fmt->{default};
>  	    $res->{$opt} = $fmt->{default} if exists $fmt->{default};

The checks seems now redundant and possible incomplete, just because now all
are always optional doesn't mean that it is the case in the future. We could do:

raise_param_exc(...) if !$fmt->{optional} || !exists $fmt->{default};

$res->{$opt} = $fmt->{default};


Need to rethink this, as is it's not correct (thanks for noticing) but I think
that I thought about something else here, damn brain..