From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
"Max R. Carrara" <m.carrara@proxmox.com>
Subject: Re: [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins
Date: Thu, 18 Sep 2025 08:27:17 +0200 [thread overview]
Message-ID: <0092ff33-673f-4a6a-9ae0-502b25691455@proxmox.com> (raw)
In-Reply-To: <DCOC4O4BA8L6.AF05OAHM5AMY@proxmox.com>
Am 09.09.25 um 16:22 schrieb Max R. Carrara:
> On Mon Sep 8, 2025 at 9:23 PM CEST, Thomas Lamprecht wrote:
>> Am 08.09.25 um 20:01 schrieb Max R. Carrara:
>>> GUI Support for Custom Storage Plugins
>>> ======================================
>>>
>>> tl;dr:
>>>
>>> Add an API method to PVE::Storage::Plugin that returns the definition
>>> for the form view of custom storage plugins. This definition is used by
>>> the frontend to build the form view for creating / editing the storage
>>> config entry of the plugin. The ultimate goal here is that custom
>>> storage plugin devs don't have to (and also *must not*) ever touch
>>> JavaScript to make their plugins show up in the GUI.
>>
>> Did not check the changes in full, but I see no mentioning of the ACME
>> DNS plugins, which already provide such a mechanism, albeit a bit lower
>> level with no real control how the fields get layouted. Did you check
>> that out and maybe already re-used the underlying parts of the
>> implementation but just not referenced it, or is this something
>> completely new/custom?
>
> This is something completely new, as the code regarding the ACME DNS
> plugins isn't really adaptable / applicable in this case here.
>
> When Aaron and I brainstormed the whole thing, he did mention that we
> already do a somewhat similar thing with the ACME DNS plugins and
> suggested that I might want to check that out first. Dominik then also
> mentioned it, but suggested that I probably shouldn't implement it the
> way we did for the ACME DNS plugins, so I decided to not look at that
> code at all.
>
> However, Dominik and I just went through the ACME GUI stuff for good
> measure and found two main reasons why the functionality can't really be
> reused:
>
> 1. The ACME DNS plugin [schema] is too simple and doesn't really
> translate to storage plugins.
As mentioned off list, the general approach to noticing that is to
first evaluate if it can be extended, not jumping straight to NIH.
As tldr/top-level points that this implementation really should
focus on, not only to avoid a maintenance burden we got to support
basically forever (or have to pay even more to move away from) but
also to have any realistic chance to get in soon are:
- We got a SectionConfig here as base, that provides update and
create schemas for the underlying data required to configure a
plugin. We already derive the CLI UI from that, so it only makes
sense to also derive the web UI from that, it's the essential base
schema definition after all and one of our core principles is to
reuse (and extend if needed) this as wide as possible to ensure
having a single source of truth that is much harder to get outdated
than *any* other approach.
So, basically, if we ever got a schema defined somewhere and the
task is that one configures entities that are backed by that schema
then it's basically should be the last resort to create a second
system for doing that.
- The first iteration of this must be a MVP (minimum viable product).
It must be simple and the only core feature it must provide is that
a user can enter all relevant configuration values without touching
the CLI or API "manually".
This could even be just a "dumb" list of free form text fields,
basically what the ACME/DNS plugin stuff is. But we can to a bit
better thanks to having the schema and being able to add the most
critical properties required to provide a better UX (more on them
below). But even that "doing better" part I'd put in separate
commits building on this base idea, as this can really be done
in steps. Getting the existing schema to the frontend is the core
here, so I think it'd be best to focus on that first.
- To get the "dumb form" the storage plugin ideally should not have
to do anything. We can still add some basic requirements (like link
to upstream storage plugin documentation), plugin authors already
define the schema. This can still be extended in the long run, it's
not set in stone and much easier to add the few actual missing
features (ideally in a declarative way) over more complex separate
mechanisms.
- finally: Storage do not get add highly frequent, for lots (most?)
setups it's done once on initial cluster setup, simply not worth it
to start out this complex even if our schema would be much more
limited, but it will allow for the basic UX niceties that make
will allow us a nicer implementation than what ACME DNS provides,
which is also not limited by the basic approach, but mostly because
nobody invested more time in it, not even in adding the schema
for more plugins (IIRC I did most of them...).
That (minus some details) should provide the base directions for an
initial integration.
> In particular, the [schema] is static, with all fields except one being
> of type "string". Things like checkboxes and dynamic dropdown selection
> boxes aren't really accounted for, since they're not needed.
As mentioned, dynamic drop down selection are not relevant for now.
For most things they are not practical for external plugins as basically
all of them are network targets and thus one cannot query some properties
on the fly of the target. Static For configuration combinations that
>
> 2. The ACME DNS API [edit window] is designed for editing on the fly.
>
> The entire [schema] is fetched at once from the backend. The window
> remembers which DNS API the user selected and stores the KV-pairs in the
> background. So when you switch back and forth between e.g. "Cloudflare
> Managed DNS" and "Alibaba Cloud DNS", the values you entered are
> retained.
Yes, that's generally a good thing and the switching does not apply at
all to the storage system, where one cannot change the storage type
during creating or editing a storage entry. And clearing the fields
between switching would be trivial to do, so really not sure how that
could be used as argument?
>
> If you then switch to an API without any fields in the schema, the
> existing KV-pairs of other plugins are inserted into the fallback text
> area. If you select an API with fields again, the content of the text
> area is parsed back into the KV-pairs.
>
> So, also not something that's really applicable / reusable for storage
> plugins.
I think there is a serious misconception on what reuse means, or at least
way to literal. One can reuse smaller parts of a thing, one can also reuse
semantics to provide a coherent system and not a dozens of NIH ones.
If it was just instantiating that widget without any changes it would have
been long done, but as it's obviously a bit special to ACME DNS plugins, it
naturally needs some adaption or refactoring for actual code reuse.
And ideally some of the improvements to (UX) possibilities would flow back
into ACME.
>> Further, FWICT this seems to add some parallel infra and need of
>> definitions, might we get away with "just" annotating the existing
>> schemas a bit better, i.e. add the flags for signaling secrets and
>> potentially some layout hints, as the core importance is to be able to
>> input required data to configure a storage, not that it needs to look
>> "perfect", at least not in the initial version of this implementation,
>> as adding more features and complexer layouting later on, especially
>> once we got feedback from integrators and see how they use it.
>
> Maybe I should clarify that the core part of this RFC isn't really the
> layouting: The form view schema allows devs to specify which columns are
> used, and a column is just an object with an array of fields. The fields
> are displayed in the order in which they appear in the column. That's
> about as much layouting as one can do at the moment.
>
> What's important are the definitions of the fields themselves. Each
> field corresponds to a SectionConfig property of the storage plugin the
> form is defined for.
>
> To provide some additional context before I continue, my initial idea
> was to *just* define what SectionConfig properties are shown in the UI
> and have the rest be "inferred" in some way from the global set of
> SectionConfig properties.
>
> That quickly turned out to be a huge can of worms, raising a lot of
> questions:
>
> - Single Selection:
>
> * How would "string" type properties whose value is
> from a set of possible selection values be shown in the UI?
enums exist in our schema.
>
> Would need a hint that it's a "single-selection" field or
> something.
>
> * How would one tell the UI which possible selection values there are?
static -> enum
dynamic -> no supported as combobox in the front end, use a textfield
and the plugin can throw a telling error message.
For some specific cases we can improve it (like being able to mark a
field of being backed by the storage scan API), but definitively
something for later and not the first iteration.
>
> Hard to do with just a hint / marker, as those selection values
> can be dynamic, e.g. the possible selections for the "pool" prop of
> the ZFSPoolPlugin are determined by listing which pools are
> available via the ZFS CLI.
>
> - Multiple Selections:
>
> * Same as above, but with multiple possible selection values (usually
> properties that have a format whose name ends with "-list").
So you tell me the schema knows this already, but you cannot use it
for deriving that?!
>
> - Sensitive Properties:
>
> * How should sensitive properties be shown in the UI?
Even if (hypothetically) you show them in plain it's OK, as either the
API sends it or not, so the client is already allowed to see it, or it's
an API bug anyway.
Being able to annotate the schema with a flag to mark something as secret
is in the midterm naturally better, as long as we do not have properties
that dynamically want to be secret or not we have no problem here.
And luckily we already got 'sensitive-properties' available for plugins.
>
> We currently do not do that uniformly, e.g. the "Password" field
> of the ESXi importer always has `inputType: 'password'` whether
> you're creating or editing it, whereas the "Keyring" field of the
> RBD storage is displayed during creation but isn't shown nor
> editable when editing it (amongst other things).
Yeah, our plugins use the fact that they are native for us and can get
a first class integration, third party plugins will get not get the same
full control ever, as that's basically giving them free reign over the
users browser, which is a no go, and even your design will limit the
implementation to not be able to leverage the full capabilities of each
UI tech (ExtJS, Yew, or say Flutter, who knows what's there in the long
term future)
And that's als why I definitively won't see getting implemented through
such a mechanism, as a native UI implementation can be always made better
over a generated one, especially with specific UX/hints/...), and we
already got our implementations and are happy with them, so zero need
to switch them to something more restrictive.
>
> The list goes on; I think you get the idea. :P
No, to be hones I really do not get the idea or arguments from that.
This rather reads to me as you did not really checked out what the
existing schema capabilities might provide you for this feature, but
way to fast switched to "fully custom DSL schema like parallel
implementation" (to exaggerate) rather than can checking if there is
already info for most basic UX or how one can add the few UI specific
flags to the schema needed to provide a more than good enough UX.
Also missing the core principle of our configs being represented by
a single schema that is reused as much as possible, and definitively
must be the base source for a auto generated UI for external plugins.
I know it's fun to implement new things without having to be bothered
by existing implementations and their capabilities, but this is simply
not a greenfield project.
> So, instead of making possibly restrictive design decisions for use
> cases I'm not even aware of, I instead chose this "manual" way of
> defining the form view, sidestepping all those questions.
As talked off list: let's please start out with using the schema
directly, the first iteration might be as well consist of two patches,
one for storage to expose the create/update schema and one for manager
to use that implementing the UI (can just copy+adapt the relevant parts
from ACME for a WIP RFC). Molding that in something with a bit more UX
should not be that much work. We got many info already in the schema,
i.e. sensitive-properties, properties being "fixed" (create only) or
not (can be updated on edit). One thing that might miss is a label, but
for starters I'd not care (and IIRC we got some schema properties for
some hints, so we maybe might even reuse an existing one or add a
dedicated label one), so even some slightly improved UX to get more
frontend checks that reduce usage friction can be added now already.
The single thing I'm actually a bit worried about is storage not using
property isolation for the section configs, but if that would be indeed
a blocker then I'd prioritize it now, as that's will never get easier
to change, on the contrary, the more external devs, plugins and storage
features we got, the harder it will be to change.
> *For now* this also seems to be the simpler, and more importantly, more
> maintainable option, since things related to the UI aren't coupled to
> the SectionConfig definitions (besides specifying what property a field
> corresponds to). I think that this approach makes it overall easier for
> third-party plugin devs to integrate their plugin into the GUI, and it's
> something we could eventually also adopt for most of our own plugins.
There is always a coupling between the section config schema and the UI,
as the latter *is* just an interface for the former, that's nothing to
argue about, it simply *has to be*.
Hiding that, like you try to do here, is just adding hidden coupling,
which has some good potential to become a PITA to manage for us and also
devs using this.
>> As it's much easier to see what's really required and what might be
>> rather a (not much used) headache to maintain support for in ExtJS
>> and a future Yew based UI. I.e., I'd be totally fine if the initial
>> version would basically look like what the ACME DNS plugin UI does,
>> focusing purely on required parameters, data types and validation
>> over layout.
>
> I totally get your point here, and the focus is on exactly
> that—whether or not the columns are in the schema doesn't really make
> any difference IMHO.
Yeah, don't focus on the columns, that's the least of our problems
here.
In conclusion: let's get a simple implementation out sooner that has
minimal code changes, we can always extend that, both in schema
capabilities or even in overall approach. It's *much* easier to keep
backward compatibility support for something as simple that we have
to support anyway (i.e., the section config schema), so even if it turns
out that it just has to be some dedicated UI specific schema stuff with
lua scripting and what not, we could still do it, but given the existing
schema state and capability I really do not see any need for starting
out with anything like that.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-09-18 6:27 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-08 18:00 Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 01/12] plugin: meta: add package PVE::Storage::Plugin::Meta Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 02/12] api: Add 'plugins/storage' and 'plugins/storage/{plugin}' paths Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 03/12] plugin: meta: introduce 'short-name' Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 04/12] plugin: views: add package PVE::Storage::Plugin::Views Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 05/12] plugin: add new plugin API method `get_form_view()` Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 06/12] plugin: meta: add metadata regarding views in API Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 07/12] api: views: add paths regarding storage plugin views Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-storage master v1 08/12] plugin: zfspool: add 'short-name' and form view for ZFS pool plugin Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-manager master v1 09/12] api: handle path 'plugins/storage' through its package Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-manager master v1 10/12] ui: storage: add CustomBase.js Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-manager master v1 11/12] ui: storage: support custom storage plugins in Datacenter > Storage Max R. Carrara
2025-09-08 18:00 ` [pve-devel] [RFC pve-manager master v1 12/12] ui: storage: use `Ext.Msg.alert()` instead of throwing an exception Max R. Carrara
2025-09-08 19:23 ` [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Thomas Lamprecht
2025-09-09 14:21 ` Max R. Carrara
2025-09-18 6:27 ` Thomas Lamprecht [this message]
2025-09-18 18:32 ` Max R. Carrara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0092ff33-673f-4a6a-9ae0-502b25691455@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=m.carrara@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.