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 10:47 AM CET, Max R. Carrara wrote:
> 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!)

Thanks for the quick feedback!

Yeah, but for something integral as the section config adding some test
cases documenting the changes would be great, I'll do that for the v2!

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

Thanks for the insights, I haven't looked too closely at the json schema
draft yet!

You're right, even though it seems that the draft doesn't specify any
behavior regarding our `optional` property, my proposed change in our
json schema code would break assumptions around our code base and with
all the `min*` properties in mind it indeed would be inconsistent if
those `min*` properties default to `0`. So let's discard that idea.

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