On Wed, Mar 11, 2026 at 01:26:18PM +0100, Daniel Kral wrote:
> parse_config(...) warns about and skips sections, which do not specify
> all required properties according to their schema, if $allow_unknown is
> not set.
>
> Meanwhile, write_config(...) will not write the config line for some
> required options, as format_config_line(...) does return an empty string
> for empty array properties or other empty non-boolean properties.
>
> To ensure that required options are always written to the config to not
> be skipped by parse_config(...), check whether format_config_line(...)
> produces any output.
>
> Signed-off-by: Daniel Kral <[email protected]>
> ---
> This came up while fixing [0] as I noticed that at least for required
> options with the '*-list' format, the parameter can still be an empty
> string / list. AFAICT this is because we check for definedness of the
> property value only.
>
> Even though I considered changing this in PVE::JSONSchema first, it
> could break existing code where this behavior is currently expected.
> Essentially, the check would need to be specified for types, e.g., to
> ensure that setting '0' for booleans, integer and numbers is still
> interpreted as a set required property.
>
> The behavior is much more apparent for section configs, where
> parse_config(...) expects these properties to be written in the config,
> while write_config(...) will drop these for empty strings and will
> result in warn log messages for any subsequent reads with $allow_unknown
> set. Therefore I propose the change only for section config for now.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=7399
>
> src/PVE/SectionConfig.pm | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
> index 84ff81a..d0332f9 100644
> --- a/src/PVE/SectionConfig.pm
> +++ b/src/PVE/SectionConfig.pm
> @@ -1572,11 +1572,12 @@ sub write_config {
> next if $opts->{$k}->{optional};
> $done_hash->{$k} = 1;
> my $v = $scfg->{$k};
> - die "section '$sectionId' - missing value for required option
> '$k'\n"
> - if !defined($v);
> $v = $class->encode_value($type, $k, $v);
Note that this also changes the public section config API by requiring
`encode_value()` to deal with the `$value` parameter being `undef` -
previously this would not happen here.
Its description would need to be updated, and its example as well, since
it only checks the `$key` before then dereferencing `$value` as a hash.
A quick grep shows a bunch of unprotected uses:
- PVE::Storage::Plugin for `nodes` or `content{,-dirs}`
- PVE::SDN::Zones::Plugin
- PVE::ACME::Challenge
- PVE::HA::Rules
- PVE::HA::Groups
- plugins for PVE::Job::Registry
> my $prop = $class->get_property_schema($type, $k);
> - $data .= format_config_line($prop, $k, $v);
> + my $cfg_line = format_config_line($prop, $k, $v);
> + die "section '$sectionId' - missing value for required option
> '$k'\n"
> + if !$cfg_line;
> + $data .= $cfg_line;
> }
>
> for my $k (@option_keys) {
> --
> 2.47.3
