osbuild / images

Image builder image definition library
Apache License 2.0
23 stars 53 forks source link

Disk customization validation #1065

Open achilleas-k opened 5 days ago

achilleas-k commented 5 days ago

With the new DiskCustomization we have two levels of validation:

This separation is useful for users of the library that may want to validate one but not the other. Currently, Validate() is run automatically when calling customizations.GetPartitioning(), but the layout constraints aren't checked. Also, the Validate() function is called in the NewCustomPartitionTable() function to check for the inconsistencies but doesn't disallow creating a partition table that violates the layout constraints.

There is a concern that this separation might cause issues with validations being dropped because callers might forget or not realise that there are multiple validations to run.

I should also add that there is a third validation, CheckMountpointPolicy(), for checking if custom mountpoints are allowed for a given image type.

We should make it easier to validate customizations, harder to skip a specific validation, but also make it possible when needed.

Proposals:

  1. Add options/arguments to the getter or the validator.
  2. Validate everything in the getter function but also keep validator functions separate, so any caller can, if needed, get the customizations without calling the getter and call each validation separately.
  3. Return typed errors and let the caller handle each validation failure type.
  4. Validate everything always and separate it down the line if the need arises.
bcl commented 5 days ago

I'd add disable options to the validator. This way there's one things to call, and by default it validates everything. And when someone copy and pastes an example they don't have to know about optional other validators :) If users need it to be less strict they can disable the layout and or constraint validation.