* [pve-devel] [RFC pve-storage master v1 01/12] plugin: meta: add package PVE::Storage::Plugin::Meta
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
@ 2025-09-08 18:00 ` 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
` (11 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
This package is used to retrieve general metadata about plugins.
Add this package in order to keep code concerning the retrieval of
storage plugin metadata in one place instead of mixing the code into
`PVE::Storage` and `PVE::Storage::Plugin`.
At the moment, plugin metadata includes the plugin's kind (inbuilt or
custom), its supported content types and formats, and what properties
it declares as sensitive.
Since plugin metadata (such as the returned hash by the `plugindata()`
method, for example) is static, cache the metadata of all plugins
after the first call to either `get_plugin_metadata()` or
`get_plugin_metadata_all()`.
The public subroutines (deep-)copy their returned data to prevent any
accidental modification, as hashrefs aren't supported by the
'use constant' Perl core pragma. This isn't the most optimal way to
do this; as a potential alternative `Readonly` [0] could be used
instead, but I didn't want to pull in another dependency at the
moment.
[0]: https://metacpan.org/pod/Readonly
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/Storage/Makefile | 1 +
src/PVE/Storage/Plugin/Makefile | 10 ++
src/PVE/Storage/Plugin/Meta.pm | 168 ++++++++++++++++++++++++++++++++
3 files changed, 179 insertions(+)
create mode 100644 src/PVE/Storage/Plugin/Makefile
create mode 100644 src/PVE/Storage/Plugin/Meta.pm
diff --git a/src/PVE/Storage/Makefile b/src/PVE/Storage/Makefile
index a67dc25..ca687b6 100644
--- a/src/PVE/Storage/Makefile
+++ b/src/PVE/Storage/Makefile
@@ -19,5 +19,6 @@ SOURCES= \
.PHONY: install
install:
make -C Common install
+ make -C Plugin install
for i in ${SOURCES}; do install -D -m 0644 $$i ${DESTDIR}${PERLDIR}/PVE/Storage/$$i; done
make -C LunCmd install
diff --git a/src/PVE/Storage/Plugin/Makefile b/src/PVE/Storage/Plugin/Makefile
new file mode 100644
index 0000000..ca82517
--- /dev/null
+++ b/src/PVE/Storage/Plugin/Makefile
@@ -0,0 +1,10 @@
+SOURCES = Meta.pm \
+
+
+INSTALL_PATH = ${DESTDIR}${PERLDIR}/PVE/Storage/Plugin
+
+.PHONY: install
+install:
+ set -e && for SOURCE in ${SOURCES}; \
+ do install -D -m 0644 $$SOURCE ${INSTALL_PATH}/$$SOURCE; \
+ done
diff --git a/src/PVE/Storage/Plugin/Meta.pm b/src/PVE/Storage/Plugin/Meta.pm
new file mode 100644
index 0000000..6d0cb51
--- /dev/null
+++ b/src/PVE/Storage/Plugin/Meta.pm
@@ -0,0 +1,168 @@
+package PVE::Storage::Plugin::Meta;
+
+use v5.36;
+
+use Carp qw(croak confess);
+use Storable qw(dclone);
+
+use PVE::Storage;
+use PVE::Storage::Plugin;
+
+use Exporter qw(import);
+
+our @EXPORT_OK = qw(
+ plugin_kinds
+ plugin_content_types
+ plugin_formats
+ get_plugin_metadata
+ get_plugin_metadata_all
+);
+
+=head1 NAME
+
+PVE::Storage::Plugin::Meta - Retrieving Storage Plugin Metadata
+
+=head1 DESCRIPTION
+
+=for comment
+TODO
+
+=cut
+
+my $PLUGIN_KINDS = [
+ 'builtin', 'custom',
+];
+
+# Note: 'none' isn't included here since it's an internal marker content type.
+my $PLUGIN_CONTENT_TYPES = [
+ 'images', 'rootdir', 'vztmpl', 'iso', 'backup', 'snippets', 'import',
+];
+
+my $PLUGIN_FORMATS = [
+ 'raw', 'qcow2', 'vmdk', 'subvol',
+];
+
+my $DEFAULT_PLUGIN_FORMAT = 'raw';
+
+sub plugin_kinds() {
+ return [$PLUGIN_KINDS->@*];
+}
+
+sub plugin_content_types() {
+ return [$PLUGIN_CONTENT_TYPES->@*];
+}
+
+sub plugin_formats() {
+ return [$PLUGIN_FORMATS->@*];
+}
+
+my $plugin_metadata = undef;
+
+my sub assemble_plugin_metadata_content($plugin) {
+ confess '$plugin is undef' if !defined($plugin);
+
+ my $content_metadata = {
+ supported => [],
+ default => [],
+ };
+
+ my $plugindata = $plugin->plugindata();
+
+ return $content_metadata if !defined($plugindata->{content});
+
+ my $supported = $plugindata->{content}->[0];
+ my $default = $plugindata->{content}->[1];
+
+ for my $content_type ($PLUGIN_CONTENT_TYPES->@*) {
+ if (defined($supported->{$content_type})) {
+ push($content_metadata->{supported}->@*, $content_type);
+ }
+
+ if (defined($default->{$content_type})) {
+ push($content_metadata->{default}->@*, $content_type);
+ }
+ }
+
+ return $content_metadata;
+}
+
+my sub assemble_plugin_metadata_format($plugin) {
+ confess '$plugin is undef' if !defined($plugin);
+
+ my $plugindata = $plugin->plugindata();
+
+ if (!defined($plugindata->{format})) {
+ return {
+ supported => [$DEFAULT_PLUGIN_FORMAT],
+ default => $DEFAULT_PLUGIN_FORMAT,
+ };
+ }
+
+ my $format_metadata = {
+ supported => [],
+ default => $plugindata->{format}->[1],
+ };
+
+ my $supported = $plugindata->{format}->[0];
+
+ for my $format ($PLUGIN_FORMATS->@*) {
+ if (defined($supported->{$format})) {
+ push($format_metadata->{supported}->@*, $format);
+ }
+ }
+
+ return $format_metadata;
+}
+
+my sub assemble_plugin_metadata() {
+ return if defined($plugin_metadata);
+
+ $plugin_metadata = {};
+ my $all_types = PVE::Storage::Plugin->lookup_types();
+
+ for my $type ($all_types->@*) {
+ my $plugin = PVE::Storage::Plugin->lookup($type);
+
+ $plugin = "$plugin";
+
+ my $kind = 'builtin';
+ $kind = 'custom' if $plugin =~ m/^PVE::Storage::Custom::/;
+
+ my $metadata = {
+ type => $type,
+ module => $plugin,
+ kind => $kind,
+ };
+
+ $metadata->{content} = assemble_plugin_metadata_content($plugin);
+ $metadata->{format} = assemble_plugin_metadata_format($plugin);
+
+ my $sensitive_properties = $plugin->plugindata()->{'sensitive-properties'} // {};
+
+ $metadata->{'sensitive-properties'} =
+ [grep { $sensitive_properties->{$_} } sort keys $sensitive_properties->%*];
+
+ $plugin_metadata->{$type} = $metadata;
+ }
+
+ return;
+}
+
+sub get_plugin_metadata {
+ my ($plugin_type) = @_;
+
+ croak "\$plugin_type is undef" if !defined($plugin_type);
+
+ assemble_plugin_metadata() if !defined($plugin_metadata);
+
+ return dclone($plugin_metadata->{$plugin_type}) if exists($plugin_metadata->{$plugin_type});
+ return undef;
+}
+
+sub get_plugin_metadata_all {
+ assemble_plugin_metadata() if !defined($plugin_metadata);
+
+ return dclone($plugin_metadata);
+}
+
+1;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 02/12] api: Add 'plugins/storage' and 'plugins/storage/{plugin}' paths
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins 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 ` 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
` (10 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
Add these paths in order to expose plugin metadata via the API.
Both paths use a common JSON schema for plugin metadata;
'plugins/storage' lists the metadata for all plugins, whereas
'plugins/storage/{plugin}' returns the metadata for a single plugin.
The queried metadata is validated against the JSON schema for each API
call. This is rather suboptimal and should be done via tests instead,
but is kept in regardless to keep this RFC short.
What permissions the two paths should use has not yet been decided.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/API2/Makefile | 1 +
src/PVE/API2/Plugins/Makefile | 18 +++
src/PVE/API2/Plugins/Storage/Config.pm | 168 +++++++++++++++++++++++++
src/PVE/API2/Plugins/Storage/Makefile | 17 +++
4 files changed, 204 insertions(+)
create mode 100644 src/PVE/API2/Plugins/Makefile
create mode 100644 src/PVE/API2/Plugins/Storage/Config.pm
create mode 100644 src/PVE/API2/Plugins/Storage/Makefile
diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile
index fe316c5..01b7b28 100644
--- a/src/PVE/API2/Makefile
+++ b/src/PVE/API2/Makefile
@@ -5,3 +5,4 @@ install:
install -D -m 0644 Disks.pm ${DESTDIR}${PERLDIR}/PVE/API2/Disks.pm
make -C Storage install
make -C Disks install
+ make -C Plugins install
diff --git a/src/PVE/API2/Plugins/Makefile b/src/PVE/API2/Plugins/Makefile
new file mode 100644
index 0000000..b235d67
--- /dev/null
+++ b/src/PVE/API2/Plugins/Makefile
@@ -0,0 +1,18 @@
+SOURCES =
+
+
+SUBDIRS = Storage \
+
+
+INSTALL_PATH = ${DESTDIR}${PERLDIR}/PVE/API2/Plugins
+
+
+.PHONY: install
+install:
+ set -e && for SOURCE in ${SOURCES}; \
+ do install -D -m 0644 $$SOURCE ${INSTALL_PATH}/$$SOURCE; \
+ done
+ set -e && for SUBDIR in ${SUBDIRS}; \
+ do make -C $$SUBDIR install; \
+ done
+
diff --git a/src/PVE/API2/Plugins/Storage/Config.pm b/src/PVE/API2/Plugins/Storage/Config.pm
new file mode 100644
index 0000000..064aec9
--- /dev/null
+++ b/src/PVE/API2/Plugins/Storage/Config.pm
@@ -0,0 +1,168 @@
+package PVE::API2::Plugins::Storage::Config;
+
+use v5.36;
+
+use HTTP::Status qw(:constants);
+
+use PVE::Exception qw(raise);
+use PVE::JSONSchema;
+use PVE::Storage;
+use PVE::Storage::Plugin;
+use PVE::Storage::Plugin::Meta qw(
+ plugin_kinds
+ plugin_content_types
+ plugin_formats
+ get_plugin_metadata
+ get_plugin_metadata_all
+);
+use PVE::Tools qw(extract_param);
+
+use PVE::RESTHandler;
+use base qw(PVE::RESTHandler);
+
+my $PLUGIN_METADATA_SCHEMA = {
+ type => 'object',
+ properties => {
+ type => {
+ type => 'string',
+ enum => PVE::Storage::Plugin->lookup_types(),
+ optional => 0,
+ },
+ kind => {
+ type => 'string',
+ enum => plugin_kinds(),
+ optional => 0,
+ },
+ content => {
+ type => 'object',
+ optional => 0,
+ properties => {
+ supported => {
+ type => 'array',
+ items => {
+ type => 'string',
+ enum => plugin_content_types(),
+ },
+ },
+ default => {
+ type => 'array',
+ items => {
+ type => 'string',
+ enum => plugin_content_types(),
+ },
+ },
+ },
+ },
+ format => {
+ type => 'object',
+ optional => 0,
+ properties => {
+ supported => {
+ type => 'array',
+ items => {
+ type => 'string',
+ enum => plugin_formats(),
+ },
+ },
+ default => {
+ type => 'string',
+ enum => plugin_formats(),
+ },
+ },
+ },
+ 'sensitive-properties' => {
+ type => 'array',
+ optional => 0,
+ items => {
+ type => 'string',
+ },
+ },
+ },
+};
+
+# plugins/storage
+
+__PACKAGE__->register_method({
+ name => 'index',
+ path => '',
+ method => 'GET',
+ description => 'List all available storage plugins and their metadata.',
+ permissions => {
+ # TODO: perms
+ description => "",
+ user => 'all',
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ kind => {
+ description => "Only list built-in or custom storage plugins.",
+ type => 'string',
+ enum => plugin_kinds(),
+ optional => 1,
+ },
+ },
+ },
+ returns => {
+ type => 'array',
+ items => $PLUGIN_METADATA_SCHEMA,
+ },
+ code => sub($param) {
+ my $param_kind = extract_param($param, 'kind');
+
+ my $result = [];
+
+ my $plugin_metadata = get_plugin_metadata_all();
+
+ for my $type (sort keys $plugin_metadata->%*) {
+ my $type_info = $plugin_metadata->{$type};
+
+ # TODO: run in tests instead?
+ PVE::JSONSchema::validate($type_info, $PLUGIN_METADATA_SCHEMA);
+
+ my $kind = $type_info->{kind};
+
+ next if defined($param_kind) && $kind ne $param_kind;
+
+ push($result->@*, $type_info);
+ }
+
+ return $result;
+ },
+});
+
+# plugins/storage/{plugin}
+
+__PACKAGE__->register_method({
+ name => 'info',
+ path => '{plugin}',
+ method => 'GET',
+ description => "Show general information and metadata of a storage plugin.",
+ permissions => {
+ # TODO: perms
+ description => "",
+ user => 'all',
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ plugin => {
+ type => 'string',
+ },
+ },
+ },
+ returns => $PLUGIN_METADATA_SCHEMA,
+ code => sub($param) {
+ my $param_type = extract_param($param, 'plugin');
+
+ my $plugin_metadata = get_plugin_metadata($param_type)
+ or raise("Plugin '$param_type' not found", code => HTTP_NOT_FOUND);
+
+ # TODO: run in tests for each plugin instead?
+ PVE::JSONSchema::validate($plugin_metadata, $PLUGIN_METADATA_SCHEMA);
+
+ return $plugin_metadata;
+ },
+});
+
+1;
diff --git a/src/PVE/API2/Plugins/Storage/Makefile b/src/PVE/API2/Plugins/Storage/Makefile
new file mode 100644
index 0000000..73875cf
--- /dev/null
+++ b/src/PVE/API2/Plugins/Storage/Makefile
@@ -0,0 +1,17 @@
+SOURCES = Config.pm \
+
+
+SUBDIRS =
+
+
+INSTALL_PATH = ${DESTDIR}${PERLDIR}/PVE/API2/Plugins/Storage
+
+.PHONY: install
+install:
+ set -e && for SOURCE in ${SOURCES}; \
+ do install -D -m 0644 $$SOURCE ${INSTALL_PATH}/$$SOURCE; \
+ done
+ set -e && for SUBDIR in ${SUBDIRS}; \
+ do make -C $$SUBDIR install; \
+ done
+
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 03/12] plugin: meta: introduce 'short-name'
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins 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 ` 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
` (9 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
'short-name' is a new key that can be defined in a plugin's
`plugindata()`, containing the plugin's "colloquially used" or
abbreviated name.
For example:
- ZFS pool plugin / 'zfspool' --> "ZFS"
- Directory plugin / 'dir' --> "Directory"
- LVM thin pool plugin / 'lvmthin' --> "LVM-Thin"
- ... and so on.
This key is added so that custom storage plugins can define how they
are named in user interfaces and whatnot, instead of just using their
`type()`.
Optionally return 'short-name' as part of the 'plugins/storage' and
'plugins/storage/{plugin}' endpoints.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/API2/Plugins/Storage/Config.pm | 4 ++++
src/PVE/Storage/Plugin/Meta.pm | 3 +++
2 files changed, 7 insertions(+)
diff --git a/src/PVE/API2/Plugins/Storage/Config.pm b/src/PVE/API2/Plugins/Storage/Config.pm
index 064aec9..2160e4e 100644
--- a/src/PVE/API2/Plugins/Storage/Config.pm
+++ b/src/PVE/API2/Plugins/Storage/Config.pm
@@ -33,6 +33,10 @@ my $PLUGIN_METADATA_SCHEMA = {
enum => plugin_kinds(),
optional => 0,
},
+ 'short-name' => {
+ type => 'string',
+ optional => 1,
+ },
content => {
type => 'object',
optional => 0,
diff --git a/src/PVE/Storage/Plugin/Meta.pm b/src/PVE/Storage/Plugin/Meta.pm
index 6d0cb51..561c01b 100644
--- a/src/PVE/Storage/Plugin/Meta.pm
+++ b/src/PVE/Storage/Plugin/Meta.pm
@@ -134,6 +134,9 @@ my sub assemble_plugin_metadata() {
kind => $kind,
};
+ $metadata->{'short-name'} = $plugin->plugindata()->{'short-name'}
+ if defined($plugin->plugindata()->{'short-name'});
+
$metadata->{content} = assemble_plugin_metadata_content($plugin);
$metadata->{format} = assemble_plugin_metadata_format($plugin);
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 04/12] plugin: views: add package PVE::Storage::Plugin::Views
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (2 preceding siblings ...)
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 ` 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
` (8 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
This package defines schemas and utils for storage plugin views.
A view in this context is what defines how data should be displayed to
users. Views are specified via a nested hash in Perl, which is then
serialized into JSON.
Right now, the only such view that can be defined is a "form" view.
A form view is simply the representation of a single record; in the
context of storage plugins here, this would be the form that opens
when you create or edit a single storage configuration entry in the
UI.
This commit adds a versioned JSON schema for storage plugin form
views. The first version of this form view supports customizing the
"General" tab in the following ways:
- Adding various types of columns:
- Regular columns
- A "bottom" column (the wide column below the regular ones)
- Columns in the advanced subsection
- A "bottom" column in the advanced subsection
- Adding fields to those columns
- A field always corresponds to a SectionConfig property
- Fields are typed and share common attributes
(readonly, required, default)
- Specific field types have specialized attributes unique to them,
e.g. 'string' supports setting a 'display-mode', which can be
'text', 'textarea' or 'password' ('text' by default)
Because each field corresponds to a SectionConfig property, the
existing API calls for creating & editing storage config entries can
simply be reused.
The ultimate goal here is to allow custom storage plugin authors to
allow integrating their plugin into our GUI with minimal effort and
without ever having to write JavaScript code. In fact, not being able
to write JS is a hard requirement for this feature.
The form view schema will be used in further commits after this one.
Some additional context:
Most of this approach is inspired by my past experience wrangling ERP
systems [0]. The ERP system I was developing modules for in particular
defined *a lot* of data models which could all be represented via
several generalized view types (such as form, list, gantt chart,
kanban, etc.). This was possible because all of those data models
shared a common base model and consequently a common database
representation as well. While all the applications within the system
were different, the way they were built was the same.
Furthermore, the idea expressed in this commit here is a
simplification of the somewhat commonly used MVVM architectural
pattern [1], in case that helps with understanding.
[0]: https://en.wikipedia.org/wiki/Enterprise_resource_planning
[1]: https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93viewmodel
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/Storage/Plugin/Makefile | 1 +
src/PVE/Storage/Plugin/Views.pm | 242 ++++++++++++++++++++++++++++++++
2 files changed, 243 insertions(+)
create mode 100644 src/PVE/Storage/Plugin/Views.pm
diff --git a/src/PVE/Storage/Plugin/Makefile b/src/PVE/Storage/Plugin/Makefile
index ca82517..2e9b538 100644
--- a/src/PVE/Storage/Plugin/Makefile
+++ b/src/PVE/Storage/Plugin/Makefile
@@ -1,4 +1,5 @@
SOURCES = Meta.pm \
+ Views.pm \
INSTALL_PATH = ${DESTDIR}${PERLDIR}/PVE/Storage/Plugin
diff --git a/src/PVE/Storage/Plugin/Views.pm b/src/PVE/Storage/Plugin/Views.pm
new file mode 100644
index 0000000..597c657
--- /dev/null
+++ b/src/PVE/Storage/Plugin/Views.pm
@@ -0,0 +1,242 @@
+package PVE::Storage::Plugin::Views;
+
+use v5.36;
+
+use Storable qw(dclone);
+
+use PVE::JSONSchema;
+
+use Exporter qw(import);
+
+our @EXPORT_OK = qw(
+ get_form_view_schema
+);
+
+=head1 NAME
+
+PVE::Storage::Plugin::Views - Schemas and Utils for Storage Plugin Views
+
+=head1 DESCRIPTION
+
+=for comment
+TODO
+
+=cut
+
+package PVE::Storage::Plugin::Views::v1 {
+ use v5.36;
+
+ use Storable qw(dclone);
+
+ use PVE::JSONSchema;
+
+ my $FIELD_TYPES = [
+ 'boolean', 'integer', 'number', 'string', 'selection',
+ ];
+
+ my $ATTRIBUTES_COMMON = {
+ required => {
+ type => 'boolean',
+ optional => 1,
+ },
+ readonly => {
+ type => 'boolean',
+ optional => 1,
+ },
+ # NOTE: Overridden per field type; specified here to make clear that
+ # this is a common attribute
+ default => {
+ type => 'any',
+ optional => 1,
+ },
+ };
+
+ my $ATTRIBUTES_BOOLEAN = {
+ 'instance-types' => ['boolean'],
+ $ATTRIBUTES_COMMON->%*,
+ default => {
+ type => 'boolean',
+ optional => 1,
+ },
+ };
+
+ my $ATTRIBUTES_INTEGER = {
+ 'instance-types' => ['integer'],
+ $ATTRIBUTES_COMMON->%*,
+ default => {
+ type => 'integer',
+ optional => 1,
+ },
+ };
+
+ my $ATTRIBUTES_NUMBER = {
+ 'instance-types' => ['number'],
+ $ATTRIBUTES_COMMON->%*,
+ default => {
+ type => 'number',
+ optional => 1,
+ },
+ };
+
+ my $ATTRIBUTES_STRING = {
+ 'instance-types' => ['string'],
+ $ATTRIBUTES_COMMON->%*,
+ default => {
+ type => 'string',
+ optional => 1,
+ },
+ 'display-mode' => {
+ type => 'string',
+ enum => ['text', 'textarea', 'password'],
+ optional => 1,
+ default => 'text',
+ },
+ };
+
+ my $ATTRIBUTES_SELECTION = {
+ 'instance-types' => ['selection'],
+ $ATTRIBUTES_COMMON->%*,
+ 'selection-mode' => {
+ type => 'string',
+ enum => ['single', 'multi'],
+ optional => 1,
+ default => 'single',
+ },
+ # List of "tuples" where the first element is the selection value,
+ # and the second element is how the selection value should be displayed to the user.
+ # For example:
+ # selection_values => [
+ # ['gzip', "Compress using GZIP"],
+ # ['zstd', "Compress using ZSTD"],
+ # ['none', "No Compression"],
+ # ];
+ 'selection-values' => {
+ type => 'array',
+ optional => 0,
+ items => {
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ },
+ },
+ # The values selected by default on creation.
+ # Must exist in selection_values.
+ # If selection-mode is 'single', then only the first element is considered.
+ default => {
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ optional => 1,
+ },
+ };
+
+ my $FIELD_ATTRIBUTES_VARIANTS = [
+ $ATTRIBUTES_BOOLEAN,
+ $ATTRIBUTES_INTEGER,
+ $ATTRIBUTES_NUMBER,
+ $ATTRIBUTES_STRING,
+ $ATTRIBUTES_SELECTION,
+ ];
+
+ my $FIELD_SCHEMA = {
+ type => 'object',
+ properties => {
+ property => {
+ type => 'string',
+ optional => 0,
+ },
+ 'field-type' => {
+ type => 'string',
+ enum => $FIELD_TYPES,
+ optional => 0,
+ },
+ label => {
+ type => 'string',
+ optional => 0,
+ },
+ attributes => {
+ type => 'object',
+ 'type-property' => 'field-type',
+ oneOf => $FIELD_ATTRIBUTES_VARIANTS,
+ optional => 0,
+ },
+ },
+ };
+
+ my $COLUMN_SCHEMA = {
+ type => 'object',
+ properties => {
+ fields => {
+ type => 'array',
+ items => $FIELD_SCHEMA,
+ optional => 1,
+ },
+ },
+ };
+
+ my $FORM_VIEW_SCHEMA = {
+ type => 'object',
+ properties => {
+ general => {
+ type => 'object',
+ optional => 1,
+ properties => {
+ columns => {
+ type => 'array',
+ items => $COLUMN_SCHEMA,
+ optional => 1,
+ },
+ 'column-bottom' => {
+ $COLUMN_SCHEMA->%*, optional => 1,
+ },
+ 'columns-advanced' => {
+ type => 'array',
+ items => $COLUMN_SCHEMA,
+ optional => 1,
+ },
+ 'column-advanced-bottom' => {
+ $COLUMN_SCHEMA->%*, optional => 1,
+ },
+ },
+ },
+ },
+ };
+
+ PVE::JSONSchema::validate_schema($FORM_VIEW_SCHEMA);
+
+ sub get_form_view_schema() {
+ return dclone($FORM_VIEW_SCHEMA);
+ }
+};
+
+my $API_FORM_VIEW_SCHEMA = {
+ type => 'object',
+ properties => {
+ version => {
+ type => 'integer',
+ enum => [1],
+ optional => 0,
+ },
+ definition => {
+ type => 'object',
+ 'type-property' => 'version',
+ optional => 0,
+ oneOf => [
+ {
+ 'instance-types' => [1],
+ PVE::Storage::Plugin::Views::v1::get_form_view_schema()->%*,
+ },
+ ],
+ },
+ },
+};
+
+PVE::JSONSchema::validate_schema($API_FORM_VIEW_SCHEMA);
+
+sub get_form_view_schema() {
+ return dclone($API_FORM_VIEW_SCHEMA);
+}
+
+1;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 05/12] plugin: add new plugin API method `get_form_view()`
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (3 preceding siblings ...)
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 ` 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
` (7 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
This method returns a nested hashref containing the form view
definition for the given plugin in `$class`. The returned hashref must
correspond to the form view schema in the `::Plugin::Views` package.
Plugins that define a form view must specify so in `plugindata()`:
sub plugindata {
return {
# [...]
views => {
form => 1,
},
};
}
Returns undef by default otherwise.
`get_form_view()` takes a single parameter (besides `$class`) named
`$context`, which is a hashref that contains additional context vars
that may or may not be taken account by the method.
Currently, the only such context variable is `mode`, which can be
either 'create' or 'update', corresponding to whether a configuration
entry belonging to the plugin is being created or edited (-> updated).
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/Storage/Plugin.pm | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 2291d72..0356a25 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -684,6 +684,14 @@ sub on_delete_hook {
return undef;
}
+# TODO: POD docstring
+sub get_form_view {
+ my ($class, $context) = @_;
+
+ # no form view defined by default
+ return undef;
+}
+
sub cluster_lock_storage {
my ($class, $storeid, $shared, $timeout, $func, @param) = @_;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 06/12] plugin: meta: add metadata regarding views in API
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (4 preceding siblings ...)
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 ` 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
` (6 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
Define and expose currently possible view types and view modes in
`::Plugin::Meta`.
Add a 'views' array to the objects returned by the 'plugins/storage'
and 'plugins/storage/{plugin}' endpoints, containing all view types
that a plugin currently supports.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/API2/Plugins/Storage/Config.pm | 9 ++++++
src/PVE/Storage/Plugin/Meta.pm | 40 ++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/src/PVE/API2/Plugins/Storage/Config.pm b/src/PVE/API2/Plugins/Storage/Config.pm
index 2160e4e..60c0515 100644
--- a/src/PVE/API2/Plugins/Storage/Config.pm
+++ b/src/PVE/API2/Plugins/Storage/Config.pm
@@ -12,6 +12,7 @@ use PVE::Storage::Plugin::Meta qw(
plugin_kinds
plugin_content_types
plugin_formats
+ plugin_view_types
get_plugin_metadata
get_plugin_metadata_all
);
@@ -81,6 +82,14 @@ my $PLUGIN_METADATA_SCHEMA = {
type => 'string',
},
},
+ views => {
+ type => 'array',
+ optional => 0,
+ items => {
+ type => 'string',
+ enum => plugin_view_types(),
+ },
+ },
},
};
diff --git a/src/PVE/Storage/Plugin/Meta.pm b/src/PVE/Storage/Plugin/Meta.pm
index 561c01b..38d6279 100644
--- a/src/PVE/Storage/Plugin/Meta.pm
+++ b/src/PVE/Storage/Plugin/Meta.pm
@@ -14,6 +14,8 @@ our @EXPORT_OK = qw(
plugin_kinds
plugin_content_types
plugin_formats
+ plugin_view_types
+ plugin_view_modes
get_plugin_metadata
get_plugin_metadata_all
);
@@ -44,6 +46,14 @@ my $PLUGIN_FORMATS = [
my $DEFAULT_PLUGIN_FORMAT = 'raw';
+my $PLUGIN_VIEW_TYPES = [
+ 'form',
+];
+
+my $PLUGIN_VIEW_MODES = [
+ 'create', 'update',
+];
+
sub plugin_kinds() {
return [$PLUGIN_KINDS->@*];
}
@@ -56,6 +66,14 @@ sub plugin_formats() {
return [$PLUGIN_FORMATS->@*];
}
+sub plugin_view_types() {
+ return [$PLUGIN_VIEW_TYPES->@*];
+}
+
+sub plugin_view_modes() {
+ return [$PLUGIN_VIEW_MODES->@*];
+}
+
my $plugin_metadata = undef;
my sub assemble_plugin_metadata_content($plugin) {
@@ -114,6 +132,26 @@ my sub assemble_plugin_metadata_format($plugin) {
return $format_metadata;
}
+my sub assemble_plugin_metadata_views($plugin) {
+ confess '$plugin is undef' if !defined($plugin);
+
+ my $plugindata = $plugin->plugindata();
+
+ return [] if !defined($plugindata->{views});
+
+ my $view_metadata = [];
+
+ my $views = $plugindata->{views};
+
+ for my $view ($PLUGIN_VIEW_TYPES->@*) {
+ if (defined($views->{$view})) {
+ push($view_metadata->@*, $view);
+ }
+ }
+
+ return $view_metadata;
+}
+
my sub assemble_plugin_metadata() {
return if defined($plugin_metadata);
@@ -145,6 +183,8 @@ my sub assemble_plugin_metadata() {
$metadata->{'sensitive-properties'} =
[grep { $sensitive_properties->{$_} } sort keys $sensitive_properties->%*];
+ $metadata->{views} = assemble_plugin_metadata_views($plugin);
+
$plugin_metadata->{$type} = $metadata;
}
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 07/12] api: views: add paths regarding storage plugin views
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (5 preceding siblings ...)
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 ` 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
` (5 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
This commit adds the following paths:
- 'plugins/storage/{plugin}/views': Returns an array of objects
describing the views that a plugin currently supports. If no views
are supported, the array is empty.
- 'plugins/storage/{plugin}/views/form': Returns the form view
definition of the given plugin.
Like in an earlier commit, the form view is always validated against
its JSON schema after it was fetched. This is rather suboptimal at the
moment and will be done within tests in the future. Right now this is
left in so as to keep the RFC smaller.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/API2/Plugins/Storage/Config.pm | 7 +
src/PVE/API2/Plugins/Storage/Makefile | 1 +
src/PVE/API2/Plugins/Storage/Views.pm | 172 +++++++++++++++++++++++++
3 files changed, 180 insertions(+)
create mode 100644 src/PVE/API2/Plugins/Storage/Views.pm
diff --git a/src/PVE/API2/Plugins/Storage/Config.pm b/src/PVE/API2/Plugins/Storage/Config.pm
index 60c0515..3f57784 100644
--- a/src/PVE/API2/Plugins/Storage/Config.pm
+++ b/src/PVE/API2/Plugins/Storage/Config.pm
@@ -18,9 +18,16 @@ use PVE::Storage::Plugin::Meta qw(
);
use PVE::Tools qw(extract_param);
+use PVE::API2::Plugins::Storage::Views;
+
use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+__PACKAGE__->register_method({
+ subclass => 'PVE::API2::Plugins::Storage::Views',
+ path => '{plugin}/views',
+});
+
my $PLUGIN_METADATA_SCHEMA = {
type => 'object',
properties => {
diff --git a/src/PVE/API2/Plugins/Storage/Makefile b/src/PVE/API2/Plugins/Storage/Makefile
index 73875cf..83fab2e 100644
--- a/src/PVE/API2/Plugins/Storage/Makefile
+++ b/src/PVE/API2/Plugins/Storage/Makefile
@@ -1,4 +1,5 @@
SOURCES = Config.pm \
+ Views.pm \
SUBDIRS =
diff --git a/src/PVE/API2/Plugins/Storage/Views.pm b/src/PVE/API2/Plugins/Storage/Views.pm
new file mode 100644
index 0000000..7419fb5
--- /dev/null
+++ b/src/PVE/API2/Plugins/Storage/Views.pm
@@ -0,0 +1,172 @@
+package PVE::API2::Plugins::Storage::Views;
+
+use v5.36;
+
+use List::Util qw(any);
+
+use HTTP::Status qw(:constants);
+
+use PVE::Exception qw(raise);
+use PVE::Storage;
+use PVE::Storage::Plugin;
+use PVE::Storage::Plugin::Meta qw(
+ plugin_view_types
+ plugin_view_modes
+ get_plugin_metadata
+);
+use PVE::Storage::Plugin::Views qw(
+ get_form_view_schema
+);
+use PVE::Tools qw(extract_param);
+
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+# plugins/storage/{plugin}/views
+
+__PACKAGE__->register_method({
+ name => 'index',
+ path => '',
+ method => 'GET',
+ description => "Return available views for a plugin.",
+ permissions => {
+ # TODO: perms
+ description => "",
+ user => 'all',
+ },
+ parameters => {
+ additionalProperties => 0,
+ },
+ # NOTE: Intentionally returning an array of objects here for forward compat
+ returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ 'view-type' => {
+ type => 'string',
+ enum => plugin_view_types(),
+ optional => 0,
+ },
+ },
+ },
+ },
+ code => sub($param) {
+ my $param_type = extract_param($param, 'plugin');
+
+ my $metadata = get_plugin_metadata($param_type)
+ or raise("Plugin '$param_type' not found", code => HTTP_NOT_FOUND);
+
+ my $result = [];
+
+ for my $view_type ($metadata->{views}->@*) {
+ my $view_spec = {
+ 'view-type' => $view_type,
+ };
+
+ push($result->@*, $view_spec);
+ }
+
+ return $result;
+ },
+});
+
+# plugins/storage/{plugin}/views/form
+
+__PACKAGE__->register_method({
+ name => 'form',
+ path => 'form',
+ method => 'GET',
+ description => "Return a plugin's form view.",
+ permissions => {
+ # TODO: perms
+ description => "",
+ user => 'all',
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ plugin => {
+ type => 'string',
+ optional => 0,
+ },
+ mode => {
+ description => "The mode for which to return the view."
+ . " Can be either 'create' or 'update', depending on whether"
+ . " the storage is being created or updated (edited).",
+ type => 'string',
+ enum => plugin_view_modes(),
+ optional => 1,
+ default => 'create',
+ },
+ },
+ },
+ returns => get_form_view_schema(),
+ code => sub($param) {
+ my $param_type = extract_param($param, 'plugin');
+ my $param_mode = extract_param($param, 'mode') // 'create';
+
+ my $metadata = get_plugin_metadata($param_type)
+ or raise("Plugin '$param_type' not found", code => HTTP_NOT_FOUND);
+
+ my $views = $metadata->{views} // [];
+ raise("Plugin '$param_type' defines no views", code => HTTP_BAD_REQUEST)
+ if !scalar($views->@*);
+
+ my $has_form_view = any { $_ eq 'form' } $views->@*;
+
+ raise("Plugin '$param_type' has no form view", code => HTTP_BAD_REQUEST)
+ if !$has_form_view;
+
+ my $plugin = PVE::Storage::Plugin->lookup($param_type);
+
+ my $context = {
+ mode => $param_mode,
+ };
+
+ my $view = eval { $plugin->get_form_view($context) };
+ if (my $err = $@) {
+ raise(
+ "Error while fetching form view for plugin '$param_type': $err",
+ code => HTTP_INTERNAL_SERVER_ERROR,
+ );
+ }
+
+ if (!defined($view)) {
+ raise(
+ "Form view for plugin '$param_type' is undefined",
+ code => HTTP_INTERNAL_SERVER_ERROR,
+ );
+ }
+
+ # TODO: run in tests instead?
+ # --> test with different contexts (mode only right now)
+ eval { PVE::JSONSchema::validate($view, get_form_view_schema()); };
+ if (my $err = $@) {
+ # NOTE: left in only for debugging purposes at the moment
+ require Data::Dumper;
+
+ local $Data::Dumper::Terse = 1;
+ local $Data::Dumper::Indent = 1;
+ local $Data::Dumper::Useqq = 1;
+ local $Data::Dumper::Deparse = 1;
+ local $Data::Dumper::Quotekeys = 0;
+ local $Data::Dumper::Sortkeys = 1;
+ local $Data::Dumper::Trailingcomma = 1;
+
+ warn "Failed to validate form view of plugin '$param_type':\n$err\n";
+ warn '$context = ' . Dumper($context) . "\n";
+ warn '$view = ' . Dumper($view) . "\n";
+
+ raise(
+ "Failed to validate form view of plugin '$param_type':\n$err\n",
+ code => HTTP_INTERNAL_SERVER_ERROR,
+ );
+ }
+
+ return $view;
+ },
+});
+
+1;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-storage master v1 08/12] plugin: zfspool: add 'short-name' and form view for ZFS pool plugin
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (6 preceding siblings ...)
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 ` 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
` (4 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
This commit demonstrates how the 'short-name' key and the
`get_form_view()` method can be added to a plugin.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
src/PVE/Storage/ZFSPoolPlugin.pm | 67 ++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index d8d8d0f..6de5ee1 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -20,9 +20,13 @@ sub type {
sub plugindata {
return {
+ 'short-name' => 'ZFS',
content => [{ images => 1, rootdir => 1 }, { images => 1, rootdir => 1 }],
format => [{ raw => 1, subvol => 1 }, 'raw'],
'sensitive-properties' => {},
+ views => {
+ form => 1,
+ },
};
}
@@ -140,6 +144,69 @@ sub on_add_hook {
return;
}
+sub get_form_view {
+ my ($class, $context) = @_;
+
+ my $pool_selection_values = [];
+
+ my $cmd = ['zfs', 'list', '-t', 'filesystem', '-Hp', '-o', 'name'];
+ run_command(
+ $cmd,
+ outfunc => sub {
+ my ($line) = @_;
+
+ if ($line =~ m/ ^ (?<pool>\S+) \s* $/xn) {
+ my $pool = $+{pool};
+ push($pool_selection_values->@*, [$pool, $pool]);
+ }
+ },
+ );
+
+ $pool_selection_values = [ sort { $a->[0] cmp $b->[0] } $pool_selection_values->@* ];
+
+ my $view = {
+ version => 1,
+ definition => {
+ general => {
+ columns => [
+ {
+ fields => [
+ {
+ property => 'pool',
+ label => "Pool",
+ 'field-type' => 'selection',
+ attributes => {
+ 'selection-values' => $pool_selection_values,
+ required => 1,
+ readonly => $context->{mode} eq 'update',
+ },
+ },
+ ],
+ },
+ {
+ fields => [
+ {
+ property => 'sparse',
+ label => "Thin provision",
+ 'field-type' => 'boolean',
+ attributes => {},
+ },
+ {
+ property => 'blocksize',
+ label => "Block Size",
+ 'field-type' => 'string',
+ attributes => {},
+ },
+ ],
+ },
+ ],
+ },
+ },
+ };
+
+ return $view;
+}
+
sub path {
my ($class, $scfg, $volname, $storeid, $snapname) = @_;
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-manager master v1 09/12] api: handle path 'plugins/storage' through its package
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (7 preceding siblings ...)
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 ` 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
` (3 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
... so that all the previously added API paths can actually be called.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
PVE/API2.pm | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/PVE/API2.pm b/PVE/API2.pm
index 0c7e4654..daa78f28 100644
--- a/PVE/API2.pm
+++ b/PVE/API2.pm
@@ -17,6 +17,7 @@ use PVE::API2::Nodes;
use PVE::API2::Pool;
use PVE::API2::AccessControl;
use PVE::API2::Storage::Config;
+use PVE::API2::Plugins::Storage::Config;
__PACKAGE__->register_method({
subclass => "PVE::API2::Cluster",
@@ -43,6 +44,11 @@ __PACKAGE__->register_method({
path => 'pools',
});
+__PACKAGE__->register_method({
+ subclass => "PVE::API2::Plugins::Storage::Config",
+ path => 'plugins/storage',
+});
+
__PACKAGE__->register_method({
name => 'index',
path => '',
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-manager master v1 10/12] ui: storage: add CustomBase.js
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (8 preceding siblings ...)
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 ` 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
` (2 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
Add CustomBase.js, a copy of Base.js specifically for custom form
views of storage plugin configs.
While there is a large overlap between the files' contents, they are
still kept separate for the purposes of this RFC. This makes it
easier to differ between how custom storage plugins and inbuilt
storage plugins are handled in the GUI at the moment, until this idea
has been fleshed out more.
The main UI building logic is in `PVE.storage.CustomInputPanel`. Right
now, there are no custom fields or anything of the sort; the field's
Ext.JS code is simply stitched together piece by piece depending on
the form view definition provided.
The fields for the 'storage', 'content', 'nodes' and 'disable'
('enable') are always included in every form view and cannot be
disabled at the moment, as they exist in virtually every storage
plugin.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
www/manager6/Makefile | 1 +
www/manager6/storage/CustomBase.js | 402 +++++++++++++++++++++++++++++
2 files changed, 403 insertions(+)
create mode 100644 www/manager6/storage/CustomBase.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 85f9268d..a329d36e 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -326,6 +326,7 @@ JSSRC= \
storage/ContentView.js \
storage/BackupView.js \
storage/Base.js \
+ storage/CustomBase.js \
storage/Browser.js \
storage/CIFSEdit.js \
storage/CephFSEdit.js \
diff --git a/www/manager6/storage/CustomBase.js b/www/manager6/storage/CustomBase.js
new file mode 100644
index 00000000..9ee2417c
--- /dev/null
+++ b/www/manager6/storage/CustomBase.js
@@ -0,0 +1,402 @@
+Ext.define('PVE.panel.CustomStorageBase', {
+ extend: 'Proxmox.panel.InputPanel',
+ controller: 'storageEdit',
+
+ type: '',
+
+ onGetValues: function (values) {
+ let me = this;
+
+ if (me.isCreate) {
+ values.type = me.type;
+ } else {
+ delete values.storage;
+ }
+
+ values.disable = values.enable ? 0 : 1;
+ delete values.enable;
+
+ return values;
+ },
+
+ initComponent: function () {
+ let me = this;
+
+ me.column1.unshift(
+ {
+ xtype: me.isCreate ? 'textfield' : 'displayfield',
+ name: 'storage',
+ value: me.storageId || '',
+ fieldLabel: 'ID',
+ vtype: 'StorageId',
+ allowBlank: false,
+ },
+ {
+ xtype: 'pveContentTypeSelector',
+ cts: me.metadataForPlugin.content.supported,
+ fieldLabel: gettext('Content'),
+ name: 'content',
+ value: me.metadataForPlugin.content.default,
+ multiSelect: true,
+ allowBlank: false,
+ },
+ );
+
+ if (!me.column2) {
+ me.column2 = [];
+ }
+
+ me.column2.unshift(
+ {
+ xtype: 'pveNodeSelector',
+ name: 'nodes',
+ reference: 'storageNodeRestriction',
+ disabled: me.storageId === 'local',
+ fieldLabel: gettext('Nodes'),
+ emptyText: gettext('All') + ' (' + gettext('No restrictions') + ')',
+ multiSelect: true,
+ autoSelect: false,
+ },
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'enable',
+ checked: true,
+ uncheckedValue: 0,
+ fieldLabel: gettext('Enable'),
+ },
+ );
+
+ me.callParent();
+ },
+});
+
+Ext.define('PVE.storage.CustomBaseEdit', {
+ extend: 'Proxmox.window.Edit',
+
+ apiCallDone: function (success, response, options) {
+ let me = this;
+ if (typeof me.ipanel.apiCallDone === 'function') {
+ me.ipanel.apiCallDone(success, response, options);
+ }
+ },
+
+ initComponent: function () {
+ let me = this;
+
+ me.isCreate = !me.storageId;
+
+ if (me.isCreate) {
+ me.url = '/api2/extjs/storage';
+ me.method = 'POST';
+ } else {
+ me.url = '/api2/extjs/storage/' + me.storageId;
+ me.method = 'PUT';
+ }
+
+ me.ipanel = Ext.create(me.paneltype, {
+ title: gettext('General'),
+ type: me.type,
+ isCreate: me.isCreate,
+ storageId: me.storageId,
+ formView: me.formView,
+ metadataForPlugin: me.metadataForPlugin,
+ });
+
+ let subject = me.metadataForPlugin['short-name'] || PVE.Utils.format_storage_type(me.type);
+
+ Ext.apply(me, {
+ subject: subject,
+ isAdd: true,
+ bodyPadding: 0,
+ items: {
+ xtype: 'tabpanel',
+ region: 'center',
+ layout: 'fit',
+ bodyPadding: 10,
+ items: [
+ me.ipanel,
+ {
+ xtype: 'pveBackupJobPrunePanel',
+ title: gettext('Backup Retention'),
+ hasMaxProtected: true,
+ isCreate: me.isCreate,
+ keepAllDefaultForCreate: true,
+ showPBSHint: me.ipanel.isPBS,
+ fallbackHintHtml: gettext(
+ "Without any keep option, the node's vzdump.conf or `keep-all` is used as fallback for backup jobs",
+ ),
+ },
+ ],
+ },
+ });
+
+ if (me.ipanel.extraTabs) {
+ me.ipanel.extraTabs.forEach((panel) => {
+ panel.isCreate = me.isCreate;
+ me.items.items.push(panel);
+ });
+ }
+
+ me.callParent();
+
+ if (!me.canDoBackups) {
+ // cannot mask now, not fully rendered until activated
+ me.down('pmxPruneInputPanel').needMask = true;
+ }
+
+ if (!me.isCreate) {
+ me.load({
+ success: function (response, options) {
+ let values = response.result.data;
+ let ctypes = values.content || '';
+
+ values.content = ctypes.split(',');
+
+ if (values.nodes) {
+ values.nodes = values.nodes.split(',');
+ }
+ values.enable = values.disable ? 0 : 1;
+ if (values['prune-backups']) {
+ let retention = PVE.Parser.parsePropertyString(values['prune-backups']);
+ delete values['prune-backups'];
+ Object.assign(values, retention);
+ }
+
+ me.query('inputpanel').forEach((panel) => {
+ panel.setValues(values);
+ });
+ },
+ });
+ }
+ },
+});
+
+Ext.define('PVE.storage.CustomInputPanel', {
+ extend: 'PVE.panel.CustomStorageBase',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ onlineHelp: '',
+
+ buildFieldFromDefinition: function (me, fieldDef) {
+ let { property, label, attributes } = fieldDef;
+
+ if (property in me.visitedStorageProperties) {
+ throw (
+ `duplicate property '${property}' in form view` +
+ ` for custom storage plugin '${me.type}'`
+ );
+ }
+
+ me.visitedStorageProperties[property] = 1;
+
+ let field = {
+ name: property,
+ fieldLabel: label,
+ cbind: {},
+ };
+
+ switch (fieldDef['field-type']) {
+ case 'boolean':
+ field.xtype = 'proxmoxcheckbox';
+ field.uncheckedValue = 0;
+ break;
+
+ case 'integer':
+ field.xtype = 'proxmoxintegerfield';
+ break;
+
+ case 'number':
+ field.xtype = 'numberfield';
+ break;
+
+ case 'string':
+ switch (attributes['display-mode']) {
+ case 'text':
+ field.xtype = 'textfield';
+ break;
+ case 'textarea':
+ field.xtype = 'textarea';
+ break;
+ case 'password':
+ field.xtype = 'proxmoxtextfield';
+ field.inputType = 'password';
+ break;
+ default:
+ field.xtype = 'textfield';
+ }
+
+ break;
+
+ case 'selection':
+ field.xtype = 'proxmoxKVComboBox';
+ field.comboItems = attributes['selection-values'] || [];
+ field.autoSelect = true;
+
+ if (me.isCreate) {
+ let firstPair = attributes['selection-values'][0];
+ if (firstPair) {
+ field.value = firstPair[0];
+ }
+ }
+
+ switch (attributes['selection-mode']) {
+ case 'single':
+ field.multiSelect = false;
+ break;
+ case 'multi':
+ field.multiSelect = true;
+ break;
+ case 'default':
+ field.multiSelect = false;
+ }
+
+ break;
+
+ default:
+ field.xtype = 'displayfield';
+ break;
+ }
+
+ // **Common Attributes**
+ // required
+ if (attributes.required) {
+ field.allowBlank = false;
+ }
+
+ // readonly
+ if (attributes.readonly) {
+ switch (fieldDef['field-type']) {
+ case 'boolean':
+ field.disabled = true;
+ break;
+ case 'integer':
+ field.xtype = 'displayfield';
+ break;
+ case 'number':
+ field.xtype = 'displayfield';
+ break;
+ case 'string':
+ field.xtype = 'displayfield';
+ break;
+ case 'selection':
+ field.xtype = 'displayfield';
+ break;
+ }
+ }
+
+ // default
+ if (attributes.default && me.isCreate) {
+ switch (fieldDef['field-type']) {
+ case 'boolean':
+ field.value = Boolean(attributes.default);
+ field.checked = Boolean(attributes.default);
+ break;
+
+ case 'integer':
+ field.value = Number(attributes.default);
+ break;
+
+ case 'number':
+ field.value = Number(attributes.default);
+ break;
+
+ case 'string':
+ field.value = attributes.default;
+ break;
+
+ case 'selection':
+ switch (attributes['selection-mode']) {
+ case 'single':
+ field.value = attributes.default[0];
+ break;
+
+ case 'multi':
+ field.value = attributes.default;
+ break;
+
+ default:
+ field.value = attributes.default[0];
+ }
+ break;
+ }
+ }
+
+ return field;
+ },
+
+ buildColumnFromDefinition: function (me, columnDef) {
+ return columnDef.fields.map((fieldDef) => me.buildFieldFromDefinition(me, fieldDef));
+ },
+
+ initComponent: function () {
+ let me = this;
+
+ // TODO: take schema version into account
+
+ me.visitedStorageProperties = {
+ storage: 1,
+ content: 1,
+ notes: 1,
+ disable: 1,
+ enable: 1,
+ };
+
+ const viewDef = me.formView.definition.general;
+ const maxColumns = 2;
+ const maxAdvancedColumns = 2;
+
+ let columns = viewDef.columns ?? [];
+ let columnBottom = viewDef['column-bottom'];
+ let advancedColumns = viewDef['columns-advanced'] ?? [];
+ let advancedColumnBottom = viewDef['column-advanced-bottom'];
+
+ let columnCount = Math.min(columns.length, maxColumns);
+
+ let advancedColumnCount = Math.min(advancedColumns.length, maxAdvancedColumns);
+
+ try {
+ columns.slice(0, columnCount).map((columnDef, index) => {
+ let colName = 'column' + (index + 1);
+
+ if (!me[colName]) {
+ me[colName] = [];
+ }
+
+ me[colName] = me[colName].concat(me.buildColumnFromDefinition(me, columnDef));
+ });
+
+ if (columnBottom) {
+ if (!me.columnB) {
+ me.columnB = [];
+ }
+
+ me.columnB = me.columnB.concat(me.buildColumnFromDefinition(me, columnBottom));
+ }
+
+ advancedColumns.slice(0, advancedColumnCount).map((columnDef, index) => {
+ let colName = 'advancedColumn' + (index + 1);
+
+ if (!me[colName]) {
+ me[colName] = [];
+ }
+
+ me[colName] = me[colName].concat(me.buildColumnFromDefinition(me, columnDef));
+ });
+
+ if (advancedColumnBottom) {
+ if (!me.advancedColumnB) {
+ me.advancedColumnB = [];
+ }
+
+ me.advancedColumnB = me.advancedColumnB.concat(
+ me.buildColumnFromDefinition(me, advancedColumnBottom),
+ );
+ }
+ } catch (error) {
+ Ext.Msg.alert(gettext('Error'), error);
+ return;
+ }
+
+ me.callParent();
+ },
+});
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-manager master v1 11/12] ui: storage: support custom storage plugins in Datacenter > Storage
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (9 preceding siblings ...)
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 ` 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
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
This commit implements support for custom form views for custom
storage plugins. This is achieved by adapting the
`createStorageEditWindow()` function to request the plugin's form view
if it's a custom plugin. Whether a plugin is custom or not is first
determined by fetching the metadata of all plugins.
Furthermore, custom storage plugins now show up in the menu of the
"Add" button. If a custom plugin defines a form view, the user can
create a new storage config entry using the GUI.
The 'short-name' of custom plugins is now also displayed in the
storage config list as well as the dropdown selection list of the
"Add" button, if present. Otherwise, we fall back to just using the
plugin's type, as we were already doing.
The items in the menu of the "Add" button are now added dynamically.
This is because making requests is asynchronous. Anything else has led
to various exceptions being thrown while testing this, due to race
conditions.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
www/manager6/dc/StorageView.js | 134 +++++++++++++++++++++++++++------
1 file changed, 113 insertions(+), 21 deletions(-)
diff --git a/www/manager6/dc/StorageView.js b/www/manager6/dc/StorageView.js
index e4c6f07d..f4515c94 100644
--- a/www/manager6/dc/StorageView.js
+++ b/www/manager6/dc/StorageView.js
@@ -11,6 +11,47 @@ Ext.define(
stateId: 'grid-dc-storage',
createStorageEditWindow: function (type, sid) {
+ let me = this;
+
+ let metadataForPlugin = me.pluginMetadata[type];
+
+ if (!metadataForPlugin) {
+ Ext.Msg.alert(gettext('Error'), `Plugin '${type}' has no metadata`);
+ return;
+ }
+
+ // NOTE: zfspool is only hardcoded for demonstration purposes
+ if (metadataForPlugin.kind === 'custom' || type === 'zfspool') {
+ Proxmox.Utils.API2Request({
+ url: `/api2/extjs/plugins/storage/${type}/views/form`,
+ method: 'GET',
+ params: {
+ mode: sid ? 'update' : 'create',
+ },
+ failure: function ({ htmlStatus }) {
+ Ext.Msg.alert(gettext('Error'), htmlStatus);
+ },
+ success: function ({ result: { data } }) {
+ let formView = data;
+
+ Ext.create('PVE.storage.CustomBaseEdit', {
+ paneltype: 'PVE.storage.CustomInputPanel',
+ type: type,
+ storageId: sid,
+ canDoBackups: metadataForPlugin.content.supported.includes('backup'),
+ formView: formView,
+ metadataForPlugin: metadataForPlugin,
+ autoShow: true,
+ listeners: {
+ destroy: me.reloadStore,
+ },
+ });
+ },
+ });
+
+ return;
+ }
+
let schema = PVE.Utils.storageSchema[type];
if (!schema || !schema.ipanel) {
throw 'no editor registered for storage type: ' + type;
@@ -23,7 +64,7 @@ Ext.define(
canDoBackups: schema.backups,
autoShow: true,
listeners: {
- destroy: this.reloadStore,
+ destroy: me.reloadStore,
},
});
},
@@ -45,6 +86,63 @@ Ext.define(
let sm = Ext.create('Ext.selection.RowModel', {});
+ me.pluginMetadata = {};
+
+ let menuButtonAdd = new Ext.menu.Menu({
+ items: [],
+ });
+
+ let pushBuiltinPluginsToMenu = function () {
+ for (const [type, storage] of Object.entries(PVE.Utils.storageSchema)) {
+ console.log(`Adding builtin plugin '${type}' to Add Button`);
+ if (storage.hideAdd) {
+ continue;
+ }
+
+ menuButtonAdd.add({
+ text: PVE.Utils.format_storage_type(type),
+ iconCls: 'fa fa-fw fa-' + storage.faIcon,
+ handler: () => me.createStorageEditWindow(type),
+ });
+ }
+ };
+
+ let pushCustomPluginsToMenu = function () {
+ for (const type in me.pluginMetadata) {
+ if (!Object.hasOwn(me.pluginMetadata, type)) {
+ continue;
+ }
+
+ const metadata = me.pluginMetadata[type];
+
+ if (metadata.kind !== 'custom') {
+ continue;
+ }
+
+ menuButtonAdd.add({
+ text: metadata['short-name'] || PVE.Utils.format_storage_type(type),
+ iconCls: 'fa fa-fw fa-folder',
+ handler: () => me.createStorageEditWindow(type),
+ });
+ }
+ };
+
+ Proxmox.Utils.API2Request({
+ url: `/api2/extjs/plugins/storage`,
+ method: 'GET',
+ success: function ({ result: { data } }) {
+ data.forEach((metadata) => {
+ me.pluginMetadata[metadata.type] = metadata;
+ });
+
+ pushBuiltinPluginsToMenu();
+ pushCustomPluginsToMenu();
+ },
+ failure: function ({ htmlStatus }) {
+ Ext.Msg.alert('Error', htmlStatus);
+ },
+ });
+
let run_editor = function () {
let rec = sm.getSelection()[0];
if (!rec) {
@@ -66,23 +164,19 @@ Ext.define(
callback: () => store.load(),
});
- // else we cannot dynamically generate the add menu handlers
- let addHandleGenerator = function (type) {
- return function () {
- me.createStorageEditWindow(type);
- };
- };
- let addMenuItems = [];
- for (const [type, storage] of Object.entries(PVE.Utils.storageSchema)) {
- if (storage.hideAdd) {
- continue;
+ // value is plugin type here
+ let columnRendererType = function (value, meta, record) {
+ let metadataForPlugin = me.pluginMetadata[value];
+
+ if (metadataForPlugin && metadataForPlugin.kind === 'custom') {
+ return (
+ metadataForPlugin['short-name'] ||
+ PVE.Utils.format_storage_type(value, meta, record)
+ );
}
- addMenuItems.push({
- text: PVE.Utils.format_storage_type(type),
- iconCls: 'fa fa-fw fa-' + storage.faIcon,
- handler: addHandleGenerator(type),
- });
- }
+
+ return PVE.Utils.format_storage_type(value, meta, record);
+ };
Ext.apply(me, {
store: store,
@@ -94,9 +188,7 @@ Ext.define(
tbar: [
{
text: gettext('Add'),
- menu: new Ext.menu.Menu({
- items: addMenuItems,
- }),
+ menu: menuButtonAdd,
},
remove_btn,
edit_btn,
@@ -113,7 +205,7 @@ Ext.define(
flex: 1,
sortable: true,
dataIndex: 'type',
- renderer: PVE.Utils.format_storage_type,
+ renderer: columnRendererType,
},
{
header: gettext('Content'),
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* [pve-devel] [RFC pve-manager master v1 12/12] ui: storage: use `Ext.Msg.alert()` instead of throwing an exception
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (10 preceding siblings ...)
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 ` 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
12 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-08 18:00 UTC (permalink / raw)
To: pve-devel
... if no editor is registered for a storage plugin.
Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
---
www/manager6/dc/StorageView.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/www/manager6/dc/StorageView.js b/www/manager6/dc/StorageView.js
index f4515c94..881ecddc 100644
--- a/www/manager6/dc/StorageView.js
+++ b/www/manager6/dc/StorageView.js
@@ -54,7 +54,8 @@ Ext.define(
let schema = PVE.Utils.storageSchema[type];
if (!schema || !schema.ipanel) {
- throw 'no editor registered for storage type: ' + type;
+ Ext.Msg.alert(gettext('Error'), `No editor registered for storage type '${type}'`);
+ return;
}
Ext.create('PVE.storage.BaseEdit', {
--
2.47.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins
2025-09-08 18:00 [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins Max R. Carrara
` (11 preceding siblings ...)
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 ` Thomas Lamprecht
2025-09-09 14:21 ` Max R. Carrara
12 siblings, 1 reply; 17+ messages in thread
From: Thomas Lamprecht @ 2025-09-08 19:23 UTC (permalink / raw)
To: Proxmox VE development discussion, Max R. Carrara
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?
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.
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.
Btw. this is partially related to [0] and I thought there was
another issue tracking this, but couldn't find one from a quick search.
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3420
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins
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
0 siblings, 1 reply; 17+ messages in thread
From: Max R. Carrara @ 2025-09-09 14:21 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
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.
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.
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.
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.
>
> 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?
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?
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").
- Sensitive Properties:
* How should sensitive properties be shown in the UI?
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).
The list goes on; I think you get the idea. :P
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.
*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.
> 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.
>
> Btw. this is partially related to [0] and I thought there was
> another issue tracking this, but couldn't find one from a quick search.
Ah, thanks for pointing me towards this; I'll assign it to myself if you
don't mind.
>
> [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3420
[schema]: /usr/share/proxmox-acme/dns-challenge-schema.json
[edit window]: https://git.proxmox.com/?p=proxmox-widget-toolkit.git;a=blob;f=src/window/ACMEPluginEdit.js;h=900923b98788a8d1e6fa3f1bc2004495ac297969;hb=refs/heads/master
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins
2025-09-09 14:21 ` Max R. Carrara
@ 2025-09-18 6:27 ` Thomas Lamprecht
2025-09-18 18:32 ` Max R. Carrara
0 siblings, 1 reply; 17+ messages in thread
From: Thomas Lamprecht @ 2025-09-18 6:27 UTC (permalink / raw)
To: Proxmox VE development discussion, Max R. Carrara
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [pve-devel] [RFC pve-storage, pve-manager master v1 00/12] GUI Support for Custom Storage Plugins
2025-09-18 6:27 ` Thomas Lamprecht
@ 2025-09-18 18:32 ` Max R. Carrara
0 siblings, 0 replies; 17+ messages in thread
From: Max R. Carrara @ 2025-09-18 18:32 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On Thu Sep 18, 2025 at 8:27 AM CEST, Thomas Lamprecht wrote:
> 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.
Too keep things short, and as we've discussed off list already, I
completely agree with you here and see your point now after going
through it all again—my original goal was to actually avoid any needless
complexity by having an isolated / decoupled module, but I hadn't
realized that it would actually introduce a lot of hidden coupling and
hidden complexity regardless (amongst other issues). That was just not
obvious to me from my personal POV at all, and when checking in with a
couple others, this approach here _seemed_ quite plausible as well.
Funnily enough, while coming up with the first rough draft of all this,
I did strongly consider "inferring" everything from the existing
schemas, as in, have the plugin dev only provide the basic layout of the
properties' fields for the UI, and have the rest be derived / inferred /
whatever you want to call it. In hindsight I should've sticked with
that, but feature creep got the better of me and I took a wrong turn.
Anyway, yes, I'm going to rework all of this—also thanks a lot for the
feedback and the off-list discussion, it exposed a lot of fallacies in
my mental models of things.
>
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread