Re: [PATCH common/manager/proxmox-widget-toolkit/storage/storage-plugin-examples v3 00/17] GUI Support for Custom Storage Plugins

["Max R. Carrara" <m.carrara@proxmox.com>]

On Tue Aug 4, 2026 at 12:40 PM CEST, Thomas Ellmenreich wrote:
> First of all, I really like the idea of this, and I find it actually really
> easy to use. Without having read anything about how to configure the UI, I was
> quickly able to make changes that then also translated. That said, this also
> means that all the errors I have found could be purely caused by
> misconfiguration. In any case, here is a list of the things I have tried:
>
> - Installed the Patchseries and Plugins on a VM and tried adding Storages of
>   the 3 different Extra Plugin Types
>
> - Made manual curl requests to the different Endpoints to see the return values
>
> - Started playing around with the configuration of the 'backup-provider-dir-example'
>   Plugin to test different edge cases. Here are most of the things I did:
>
>   - Added one and then two multiline fields to see how they sorted.
>
>   - Checked that optional fields are actually optional, and vice versa.
>
>   - Made one of the multiline fields Fixed, and then checked how that showed
>     when adding the Storage and when editing it.
>
>   - Added a sensitive multiline field.
>
>   - Added a normal string field that is not optional, as well as sensitive.
>
>   - Added a multiline field to the advanced properties, while being optional
>     as well as non optional.
>
>   - Repeated some of the combinations with boolean, number and integer
>     fields.

Wew, thanks a lot for the thorough testing and the review! Glad to hear
that it's easy to use! :)

>
> Looking at the code and during these tests, I have encountered some things.
> Not everything might be important, but I will still list everything I have
> noticed. And I should also add that I'm not sure if this series is the correct
> moment to address some of the problems, as they might be more general ExtJs
> problems.
>
> - In some of the patches, there are a few strings that could be wrapped in
>   `gettext` to allow for future translation.

Actually really nice that you spotted that; will hunt those strings down
and translate them in v4. Thanks!

>
> - There seems to be an inconsistency in how required (or non optional) fields
>   show up in the UI. When a field is marked as fixed and required, it has a
>   red lining indicating the fact that it has to be set. But if it's only
>   required, there is no red line. That said, upon trying to create the storage,
>   a good error message is shown. So this is purely a UI/UX issue.

Interesting, will have a look at this too.

>
> - When adding multiline fields, they show up in the bottom section of the
>   popup UI, which is very nice. But when that field has the 'fixed' property set
>   and you then open the editing UI, instead of showing up in the bottom section,
>   the multiline field now shows up between the other properties. This is also
>   just a UI issue, it caused a second of confusion for me though, as the quite
>   wide multiline field suddenly showed up as a small fixed label, hidden
>   between other properties.

Yeah I can totally see why that might be confusing; I'll see if I can
adapt the logic there so that when a property with `multiline` format
is being used as a display field, it still gets put into the wide bottom
column (the same position as before). Nice that you spotted this,
thanks!

>
> - Adding newlines to a multiline field doesn't work and fails with this error:
>   'Property contains a line feed (500)'. I'm guessing, but I think this is
>   a problem with the `storage.cfg` file and not the UI.

Yeah that's gonna be a bit tricky, I'll have to see how multiline
strings can be stored in SectionConfig (sigh). Thanks for spotting this
as well!

>
> - Setting multiline field as sensitive seems to do nothing to the field. It
>   shows up as just a normal multiline field.

That's actually a limitation of text boxes in general, unfortunately.
The censoring cannot be applied to `<textarea>` elements, and I have
tried to look for some kind of solution in ExtJS -- I might try
customizing the new proxmoxtextarea field in some way so that this may
be taken into account. FWIW, we don't do this for the `keyring` of RBD
storages either, but might be nice to adapt that field in the UI there
too, if I do end up finding a solution.

>
> - I then added a new required string field and set that as sensitive, which
>   correctly hides the content of the field. When I then try to save the
>   storage, I get the following error: 'Missing value for required option "another-testing-prop" (500)'

If I understand correctly there, the property is treated as expected
here, right? Or does this happen when you update the config?

>
> - This is not an issue in my opinion, but I wanted to note it. Its possible
>   to have a required field in the Advanced Properties without a default. Can
>   be an annoying thing if a plugin creator sets it, but it would be a mistake
>   on the creators part.

Hmm, yeah, that surely is annoying, but I agree that it would be the
plugin author's fault.

>
> - Purely out of intuition I would have thought that the difference between
>   number and integer is that the first allows decimals and the second does no.
>   This is not the case, when adding a ',' or a '.' in both, the "decimal part"
>   just disappears.

Oh, that's interesting, actually. I will investigate further, thanks for
spotting that!

>
> On Mon Jul 27, 2026 at 4:34 PM CEST, Max R. Carrara wrote:
> > GUI Support for Custom Storage Plugins - v3
> > ===========================================
> >
> > Notable Changes Since v2
> > ------------------------
> >
> > - Drop the patternProperties RFC patches.
> >
> >   After some additional consideration, I decided that supporting
> >   patternProperties is out of scope for this series, and probably
> >   something that deserves its own series instead.
> >
> >   In turn, the 'x-advanced' and 'x-hidden' keys in the plugin property
> >   schema returned by the new API endpoint are now set through two
> >   new hashes in `plugindata()`. This means that each plugin has to
> >   define which properties it considers advanced and which should be
> >   hidden in the UI itself, just like with sensitive properties.
> >
> > - Rename the 'sensitive' key that is returned as part of the plugin
> >   property schema by the new API endpoint to 'x-sensitive'. This is done
> >   in order to make it clear that this keyword is not part of our overall
> >   JSON schema, since each plugins' properties' schemas are returned
> >   directly.
> >
> >   Note that we can still "promote" 'x-sensitive' and have it exist in
> >   the general JSON schema as 'sensitive' at some point in the future,
> >   but that's out of scope for this series.
> >
> > - Sort modules in patch #13 (thanks @Jakob for the suggestion!)
> >
> > - Update the SSHFS example plugin and make use of the new hashes in
> >   `plugindata()`, `title` keys in `properties()`, and also bump it's API
> >   version, with all changes that are necessary for this.
> >
> > Thanks again to everyone who reviewed and/or tested the previous
> > iterations of this series, as well as to those that provided feedback!
> > It's much appreciated.
> >
> > Previous Versions
> > -----------------
> >
> > rfc-v1: https://lore.proxmox.com/pve-devel/20250908180058.530119-1-m.carrara@proxmox.com/
> > rfc-v2: https://lore.proxmox.com/pve-devel/20251121165858.818307-1-m.carrara@proxmox.com/
> > v1: https://lore.proxmox.com/pve-devel/20260623143402.772452-1-m.carrara@proxmox.com/
> > v2: https://lore.proxmox.com/pve-devel/20260717154943.696411-1-m.carrara@proxmox.com/
> >
> > Summary of Changes
> > ------------------
> >
> > pve-common:
> >
> > Max R. Carrara (1):
> >   json schema: add multiline string format
> >
> >  src/PVE/JSONSchema.pm | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> >
> > pve-storage:
> >
> > Max R. Carrara (8):
> >   api: plugins/storage: add initial routes and endpoints
> >   api: plugins/storage/plugin: include schema in plugin metadata
> >   api: plugins/storage/plugin: mark sensitive properties in schema
> >   api: plugins/storage/plugin: factor plugin metadata code into helper
> >   api: plugins/storage/plugin: add plugins' 'content' to their metadata
> >   all plugins: add 'title' to properties, adapt 'description's
> >   all plugins, api: mark certain properties as advanced in plugindata()
> >   all plugins, api: mark certain properties as hidden in plugindata()
> >
> >  src/PVE/API2/Makefile                  |   1 +
> >  src/PVE/API2/Plugins/Makefile          |  18 +++
> >  src/PVE/API2/Plugins/Storage.pm        |  54 ++++++++
> >  src/PVE/API2/Plugins/Storage/Makefile  |  17 +++
> >  src/PVE/API2/Plugins/Storage/Plugin.pm | 172 +++++++++++++++++++++++++
> >  src/PVE/Storage/BTRFSPlugin.pm         |   9 ++
> >  src/PVE/Storage/CIFSPlugin.pm          |  22 +++-
> >  src/PVE/Storage/CephFSPlugin.pm        |  16 ++-
> >  src/PVE/Storage/DirPlugin.pm           |  23 +++-
> >  src/PVE/Storage/ESXiPlugin.pm          |   3 +
> >  src/PVE/Storage/ISCSIDirectPlugin.pm   |   4 +
> >  src/PVE/Storage/ISCSIPlugin.pm         |  10 +-
> >  src/PVE/Storage/LVMPlugin.pm           |  16 ++-
> >  src/PVE/Storage/LvmThinPlugin.pm       |   7 +-
> >  src/PVE/Storage/NFSPlugin.pm           |  18 ++-
> >  src/PVE/Storage/PBSPlugin.pm           |   7 +-
> >  src/PVE/Storage/Plugin.pm              |  46 +++++--
> >  src/PVE/Storage/RBDPlugin.pm           |  20 ++-
> >  src/PVE/Storage/ZFSPlugin.pm           |  20 ++-
> >  src/PVE/Storage/ZFSPoolPlugin.pm       |  13 +-
> >  20 files changed, 459 insertions(+), 37 deletions(-)
> >  create mode 100644 src/PVE/API2/Plugins/Makefile
> >  create mode 100644 src/PVE/API2/Plugins/Storage.pm
> >  create mode 100644 src/PVE/API2/Plugins/Storage/Makefile
> >  create mode 100644 src/PVE/API2/Plugins/Storage/Plugin.pm
> >
> >
> > proxmox-widget-toolkit:
> >
> > Max R. Carrara (3):
> >   form: introduce new 'proxmoxtextarea' field
> >   utils: introduce helper function getFieldDefFromPropertySchema
> >   acme: use helper to construct ExtJS fields from property schemas
> >
> >  src/Utils.js                 | 150 +++++++++++++++++++++++++++++++++++
> >  src/form/TextAreaField.js    |  47 +++++++++++
> >  src/window/ACMEPluginEdit.js |  42 +++-------
> >  3 files changed, 210 insertions(+), 29 deletions(-)
> >
> >
> > pve-manager:
> >
> > Max R. Carrara (4):
> >   api2: sort modules and API method registrations
> >   api: add API routes 'plugins' and 'plugins/storage'
> >   ui: storage view: display error when no editor for storage type exists
> >   ui: storage: add basic UI integration for custom storage plugins
> >
> >  PVE/API2.pm                        |  26 ++--
> >  PVE/API2/Makefile                  |   1 +
> >  PVE/API2/Plugins.pm                |  61 +++++++++
> >  www/manager6/Makefile              |   1 +
> >  www/manager6/dc/StorageView.js     | 132 ++++++++++++++-----
> >  www/manager6/storage/Base.js       |   2 +
> >  www/manager6/storage/CustomEdit.js | 198 +++++++++++++++++++++++++++++
> >  7 files changed, 377 insertions(+), 44 deletions(-)
> >  create mode 100644 PVE/API2/Plugins.pm
> >  create mode 100644 www/manager6/storage/CustomEdit.js
> >
> >
> > pve-storage-plugin-examples:
> >
> > Max R. Carrara (1):
> >   sshfs: Update SSHFS plugin
> >
> >  .../src/PVE/Storage/Custom/SSHFSPlugin.pm     | 27 ++++++++++++++-----
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> >
> >
> > Summary over all repositories:
> >   32 files changed, 1072 insertions(+), 116 deletions(-)
> >
> > --
> > Generated by murpp 0.12.0