Re: [pve-devel] [PATCH common] SectionConfig: fix handling unknown sections

[Fiona Ebner <f.ebner@proxmox.com>]

Am 16.08.23 um 09:41 schrieb Dominik Csapak:
> if we're parsing an unknown section, we cannot check the schema with
> `is_array` to check if it's an array type or not, thus we have to
> handle that separately.
> 
> fix this by handling data in unknown sections like an array for all
> analogous to "cb2646c7b4974e33f4148752deec71f0d589b0f3" in
> proxmox-section-config. This way we can write unknown section out again
> like we parsed it.

Thank you for tackling this!

As briefly discussed off-list, there, we only start interpreting data in
unknown sections for keys appearing multiple times as an array. While it
shouldn't make a difference if all we do with data in unknown sections
is write it back out, the fact that most sections are not arrays makes
it feel a bit more future-proof to do the same here.

> 
> we have to adapt a single test case, which is ok since that is in an
> `invalid` section of a config anyway.
> 
> This fixes an issue, where calling `qm destroy ID --purge` removed much
> of the configs ob backup jobs (since there we parse an 'unknown' section
> and run into the `is_array` error)

Reference https://forum.proxmox.com/threads/132091

> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---

(...)

> diff --git a/test/section_config_test.pl b/test/section_config_test.pl
> index 02242bc..d574150 100755
> --- a/test/section_config_test.pl
> +++ b/test/section_config_test.pl
> @@ -217,7 +217,7 @@ my $with_unknown_data = {
>  	},
>  	invalid => {
>  	    type => 'bad',
> -	    common => 'omg',
> +	    common => ['omg'],
>  	},
>      },
>      order => enum(qw(t1 t2 invalid t3)),

So this test doesn't expose the issue just because the "common" property
is already defined. Please add a second test which uses an unknown
property to expose the issue so we'd notice any future regression.