On Wed Mar 11, 2026 at 1:26 PM CET, 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
Just had a look at your patch—nice that you spotted this! I think this fix here should be fine, but I think it would be nice if you added a test: Replicate the broken behavior first, and then fix it together with this patch here. So, basically a little bit of TDD. I'm not a big fan of TDD myself, but I think in this specific case it would be beneficial for us to have a test like that—it's also documentation at the same time. Perhaps a nice spot for that would be in the SectionConfig tests directory [1] as a separate script or something—see the other scripts for how the tests in there work. (ofc you can holler at me too!) Regarding JSONSchema: I think the definedness check itself is fine—an empty array is still "something" rather than nothing, so ... We could instead implement some of the other validation keywords [2] for arrays, like e.g. `minItems`. I think an array that's `required` and also has `minItems` set to `1` would allow you to express what you initially wanted where you fixed #7399 [0], right? I just hope that this is possible in your current JSONSchema—Wolfgang and I have spoken about some things related to this a (longer) while ago; I believe it was about empty strings / zeroes as well. *That* might be a bit more complicated to solve, though and might even require a JSONSchema v2 of some sorts, but I don't have all the details in my head at the moment. [1]: https://git.proxmox.com/?p=pve-common.git;a=tree;f=test/SectionConfig;h=0967fb1eebf84b4ecad696901572fa7550d49941;hb=refs/heads/master [2]: https://json-schema.org/draft/2020-12/json-schema-validation#name-validation-keywords-for-arr > > 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); > 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) {
