public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [RFC proxmox{, -yew-comp, -datacenter-manager}/yew-mobile-gui v2 0/7] Add fallback variant to enum properties in Proxmox VE Rust API types
@ 2025-11-13 15:09 Stefan Hanreich
  2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox v2 1/4] pve-api-types: add FixedString type Stefan Hanreich
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Stefan Hanreich @ 2025-11-13 15:09 UTC (permalink / raw)
  To: pdm-devel

## Introduction

This patch series adds a new fallback variant to all enums that are
auto-generated in pve-api-types. This ensures that the addition of new enum
variants in the Proxmox VE API does not break the deserialization of Proxmox VE
API types in PDM. Rather, the unknown enum variant is now populated with the
parsed enum variant name. For that matter this patch series introduces a new
string type, FixedString. It can store up to 23 characters and is Copy, which is
the main reason for introducing this custom string type. This lets us keep Copy
in our generated enums.


## Open Questions

### Resubmitting unknown variants

I had some off list discussions with Dominik and Dietmar regarding how consumers
of the API (mainly the UI) could handle encountering unknown values when there
is the possibility of re-submission of this data. Two main options emerged:

* Resubmitting unknown values as-is, without preventing the user from doing so

This has the upside of not preventing the user from submitting perfectly fine
values if they haven't been implemented in PDM. An argument in favor would be
that the PVE API should act as the last safeguard against invalid objects in the
configuration anyway, so we could rely on PVE validation only in the case of
unknown enums.
The downside is that we need a round-trip to the Proxmox VE API and can only
show the resulting error in the UI, without any validation. It also does not
take care of situations where setting a certain, new, enum potentially requires
special behavior when submitting values.

* Showing the value, but preventing the user from re-submitting the value

This is the safest option, since it prevents any, potentially destructive,
action from the user due to the consumer not being aware of special . It also
gives the user immediate feedback in the UI, without requiring a round-trip to
the Proxmox VE instance, but of course users lose the ability to submit
perfectly valid configurations (without upgrading).


The answer is, of course, it depends on the context - but in the general case it
seems preferable to me to show the value to the user, but prevent the user from
re-submitting the value, since the UI cannot know if there is some special
handling required in case the new enum variant is used in the UI. If there is a
good argument for letting the user re-submit certain fields, then that seems
like a fine option too, but imo *iff* there's good reasoning attached to it.

What do you think? Any options that were missed?


### Storing unknown Variants

When only reading and displaying / storing data, then including unknown values
is desired behavior imo (again, in the general case) - since it allows storing
data (e.g. metrics) that isn't yet understood but could potentially be
understood perfectly fine after an upgrade of PDM.


### Unsafe FixedString

I implemented as_bytes and as_str in FixedString via the unsafe variants of the
methods and included tests that check for correct behavior of those methods.
Using the safe variants and panicking here should be fine too imo if we don't
want to go down that road. FixedString should only get used in rare, exceptional
cases anyway - and even then not in a hot path. The adaption should be trivial.


## Adapting the existing code

One difficulty when adapting the UI was identifying places in which a struct
gets returned directly from the API, deserialized (but the value isn't used
anywhere in the frontend), then serialized again and sent back to the backend.
Since the FixedString serializes indistinguishable from a String, this could
introduce situations where we submit unkown enum values when instead an error
should be shown and the user prevented from submitting a form.

I tried to look at each component in the backend / frontend and check if it uses
and potentially re-submits am enum without checking its variants for unknown
values. Since we model enums with checkboxes in the UI and have fixed values
there, this should usually work and prevent users from submitting values that
are unknown to the UI. Nevertheless, it's very much possible I missed some
places, so here some group effort in trying to identify potentially problematic
spots would be nice.


## Maintainers Notes

pve-api-types breaks building:
* proxmox-datacenter-manager
* proxmox-yew-comp
* pve-yew-mobile-gui

proxmox-datacenter-manager and pve-yew-mobile-gui additionally need the newer
version of proxmox-yew-comp to build.


## Changelog

Changes from v1 (Thanks @Wolfgang):
* remove truncate constructor
* use macros to derive some of the traits by forwarding
* cleaned up std::fmt imports
* changed error type to zero-sized struct
* add AsRef<[u8]> and Borrow<str> impls
* improved deserialize implementation

proxmox:

Stefan Hanreich (4):
  pve-api-types: add FixedString type
  pve-api-types: generate fallback variant for enums
  pve-api-types: regenerate
  pve-api-types: sdn: handle fallback variant

 pve-api-types/generator-lib/Schema2Rust.pm |   3 +
 pve-api-types/src/generated/types.rs       | 210 ++++++++++++++++
 pve-api-types/src/sdn.rs                   |   3 +-
 pve-api-types/src/types/fixed_string.rs    | 274 +++++++++++++++++++++
 pve-api-types/src/types/mod.rs             |   3 +
 5 files changed, 492 insertions(+), 1 deletion(-)
 create mode 100644 pve-api-types/src/types/fixed_string.rs


proxmox-yew-comp:

Stefan Hanreich (1):
  pve: qemu: handle fallback enum variants

 .../qemu_property/qemu_amd_sev_property.rs    |  1 +
 .../pve/qemu_property/qemu_bios_property.rs   |  1 +
 .../qemu_property/qemu_machine_property.rs    | 20 +++++++++++--------
 3 files changed, 14 insertions(+), 8 deletions(-)


proxmox-datacenter-manager:

Stefan Hanreich (1):
  tree-wide: handle new unknown enum variants

 server/src/metric_collection/rrd_task.rs | 4 ++++
 ui/src/pve/utils.rs                      | 1 +
 ui/src/widget/migrate_window.rs          | 6 ++++++
 3 files changed, 11 insertions(+)


pve-yew-mobile-gui:

Stefan Hanreich (1):
  tree-wide: handle fallback enum values

 src/pages/page_resources.rs   | 4 +++-
 src/pages/page_task_status.rs | 7 +++++++
 2 files changed, 10 insertions(+), 1 deletion(-)


Summary over all repositories:
  13 files changed, 527 insertions(+), 10 deletions(-)

-- 
Generated by git-murpp 0.8.0

_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-13 21:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-13 15:09 [pdm-devel] [RFC proxmox{, -yew-comp, -datacenter-manager}/yew-mobile-gui v2 0/7] Add fallback variant to enum properties in Proxmox VE Rust API types Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox v2 1/4] pve-api-types: add FixedString type Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox v2 2/4] pve-api-types: generate fallback variant for enums Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox v2 3/4] pve-api-types: regenerate Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox v2 4/4] pve-api-types: sdn: handle fallback variant Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox-yew-comp v2 1/1] pve: qemu: handle fallback enum variants Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 1/1] tree-wide: handle new unknown " Stefan Hanreich
2025-11-13 15:09 ` [pdm-devel] [PATCH pve-yew-mobile-gui v2 1/1] tree-wide: handle fallback enum values Stefan Hanreich
2025-11-13 20:33 ` [pdm-devel] [RFC proxmox{, -yew-comp, -datacenter-manager}/yew-mobile-gui v2 0/7] Add fallback variant to enum properties in Proxmox VE Rust API types Thomas Lamprecht
2025-11-13 21:28 ` [pdm-devel] applied-series: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal