Re: [RFC PATCH common] section config: correctly handle required options in write_config

["Daniel Kral" <d.kral@proxmox.com>]

On Thu Mar 12, 2026 at 2:06 PM CET, Wolfgang Bumiller wrote:
> On Wed, Mar 11, 2026 at 01:26:18PM +0100, Daniel Kral wrote:
>> 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.

Right, good catch! I had a bad gut feeling about moving it farther here
and only did so that the die is coupled to whether
format_config_line(...) does add a line to the config $data string.

I'd go for having both the definedness check above and the 'is the data
actually written to the config file?' check below with a more telling
error message in a v2, if there are no objections.

>
> 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