[pve-devel] [PATCH pve-manager v2 0/4] api: add return schemas

[pve-devel] [PATCH pve-manager v2 0/4] api: add return schemas

[n.frey]

From: Nicolas Frey <n.frey@proxmox.com>

these four patches add the return schemas for endpoints in
ACMEPlugin, APT, ReplicationConfig and Services using the
createSchema() function where possible.
Also tested functionality using pvesh.

Changelog:

changes since v1, thanks to @ Thomas Lamprecht:

- use createSchema where possible (and sensible)
- redundant schema removed in APT.pm and Services.pm
- correct possible values and descriptions

Nicolas Frey (4):
  api: add ACME plugin return schema
  api: add APT versions return schema
  api: add service state return schema
  api: add replication config read return schema

 PVE/API2/ACMEPlugin.pm        |  15 ++---
 PVE/API2/APT.pm               | 113 ++++++++++++++++++++--------------
 PVE/API2/ReplicationConfig.pm |  11 ++--
 PVE/API2/Services.pm          |  80 +++++++++++++++++++++++-
 4 files changed, 156 insertions(+), 63 deletions(-)

-- 
2.47.3


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

[pve-devel] [PATCH pve-manager v2 1/4] api: add ACME plugin return schema

[n.frey]

From: Nicolas Frey <n.frey@proxmox.com>

After looking at the code, I don't see any issues with using the
createSchema method. Additionally tested by comparing the output of:
- pvesh usage /cluster/acme/plugins --returns
- pvesh get /cluster/acme/plugins
confirmed that the contents match.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 PVE/API2/ACMEPlugin.pm | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/PVE/API2/ACMEPlugin.pm b/PVE/API2/ACMEPlugin.pm
index 510101aa..d52bc90b 100644
--- a/PVE/API2/ACMEPlugin.pm
+++ b/PVE/API2/ACMEPlugin.pm
@@ -50,6 +50,8 @@ my $modify_cfg_for_api = sub {
     return $plugin_cfg;
 };
 
+my $acme_challenge_type = PVE::ACME::Challenge->createSchema();
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -72,12 +74,7 @@ __PACKAGE__->register_method({
     },
     returns => {
         type => 'array',
-        items => {
-            type => "object",
-            properties => {
-                plugin => get_standard_option('pve-acme-pluginid'),
-            },
-        },
+        items => $acme_challenge_type,
         links => [{ rel => 'child', href => "{plugin}" }],
     },
     code => sub {
@@ -111,9 +108,7 @@ __PACKAGE__->register_method({
             id => get_standard_option('pve-acme-pluginid'),
         },
     },
-    returns => {
-        type => 'object',
-    },
+    returns => $acme_challenge_type,
     code => sub {
         my ($param) = @_;
 
@@ -131,7 +126,7 @@ __PACKAGE__->register_method({
         check => ['perm', '/', ['Sys.Modify']],
     },
     protected => 1,
-    parameters => PVE::ACME::Challenge->createSchema(),
+    parameters => $acme_challenge_type,
     returns => {
         type => "null",
     },
-- 
2.47.3


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

[pve-devel] [PATCH pve-manager v2 2/4] api: add APT versions return schema

[n.frey]

From: Nicolas Frey <n.frey@proxmox.com>

listed only supported debian architectures [0] in enum for 'Arch'.
listed all APT package states in enum for 'CurrentState'.

[0] https://wiki.debian.org/SupportedArchitectures

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 PVE/API2/APT.pm | 113 ++++++++++++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 46 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 0d07cf38..885b85ae 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -200,6 +200,52 @@ my $update_pve_pkgstatus = sub {
     return $pkglist;
 };
 
+my $apt_package_return_props = {
+    Arch => {
+        type => 'string',
+        description => 'Package Architecture.',
+        enum => [qw(armhf arm64 amd64 ppc64el risc64 s390x)],
+    },
+    Description => {
+        type => 'string',
+        description => 'Package description.',
+    },
+    NotifyStatus => {
+        type => 'string',
+        description => 'Version for which PVE has already sent an update notification for.',
+        optional => 1,
+    },
+    OldVersion => {
+        type => 'string',
+        description => 'Old version currently installed.',
+        optional => 1,
+    },
+    Origin => {
+        type => 'string',
+        description => "Package origin, e.g., 'Proxmox' or 'Debian'.",
+    },
+    Package => {
+        type => 'string',
+        description => 'Package name.',
+    },
+    Priority => {
+        type => 'string',
+        description => 'Package priority.',
+    },
+    Section => {
+        type => 'string',
+        description => 'Package section.',
+    },
+    Title => {
+        type => 'string',
+        description => 'Package title.',
+    },
+    Version => {
+        type => 'string',
+        description => 'New version to be updated to.',
+    },
+};
+
 __PACKAGE__->register_method({
     name => 'list_updates',
     path => 'update',
@@ -220,51 +266,7 @@ __PACKAGE__->register_method({
         type => "array",
         items => {
             type => "object",
-            properties => {
-                'Arch' => {
-                    type => 'string',
-                    description => 'Package Architecture.',
-                },
-                'Description' => {
-                    type => 'string',
-                    description => 'Human-readable package description.',
-                },
-                'NotifyStatus' => {
-                    type => 'string',
-                    description =>
-                        'Version for which PVE has already sent an update notification for.',
-                    optional => 1,
-                },
-                'OldVersion' => {
-                    type => 'string',
-                    description => 'Old version currently installed.',
-                    optional => 1,
-                },
-                'Origin' => {
-                    type => 'string',
-                    description => 'Package origin.',
-                },
-                'Package' => {
-                    type => 'string',
-                    description => 'Package name.',
-                },
-                'Priority' => {
-                    type => 'string',
-                    description => 'Package priority in human-readable form.',
-                },
-                'Section' => {
-                    type => 'string',
-                    description => 'Package section.',
-                },
-                'Title' => {
-                    type => 'string',
-                    description => 'Package title.',
-                },
-                'Version' => {
-                    type => 'string',
-                    description => 'New version to be updated to.',
-                },
-            },
+            properties => $apt_package_return_props,
         },
     },
     code => sub {
@@ -769,6 +771,25 @@ __PACKAGE__->register_method({
     },
 });
 
+$apt_package_return_props->{CurrentState} = {
+    type => 'string',
+    description => 'Current state of the package installed on the system.',
+    # Possible CurrentState variants according to AptPkg::Cache
+    enum => [qw(Installed NotInstalled UnPacked HalfConfigured HalfInstalled ConfigFiles)],
+};
+
+$apt_package_return_props->{RunningKernel} = {
+    type => 'string',
+    description => "Kernel release, only for package 'proxmox-ve'.",
+    optional => 1,
+};
+
+$apt_package_return_props->{ManagerVersion} = {
+    type => 'string',
+    description => "Version of the currently running pve-manager API server.",
+    optional => 1,
+};
+
 __PACKAGE__->register_method({
     name => 'versions',
     path => 'versions',
@@ -788,7 +809,7 @@ __PACKAGE__->register_method({
         type => "array",
         items => {
             type => "object",
-            properties => {},
+            properties => sort $apt_package_return_props,
         },
     },
     code => sub {
-- 
2.47.3


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

[pve-devel] [PATCH pve-manager v2 3/4] api: add service state return schema

[n.frey]

From: Nicolas Frey <n.frey@proxmox.com>

values for the respective states (active-state, state, unit-state)
were taken from the systemd manpages [0] for ActiveState, SubState,
and UnitFileState. With the addition of unknown and not-found to
account for logic present in the code.

[0] https://manpages.debian.org/trixie/systemd/org.freedesktop.systemd1.5.en.html

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 PVE/API2/Services.pm | 80 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Services.pm b/PVE/API2/Services.pm
index 708b6613..4b0999d1 100644
--- a/PVE/API2/Services.pm
+++ b/PVE/API2/Services.pm
@@ -137,6 +137,82 @@ my $service_state = sub {
     return $res;
 };
 
+my $service_type = {
+    'active-state' => {
+        type => 'string',
+        enum => [
+            qw(active inactive failed activating deactivating maintenance reloading refreshing unknown)
+        ],
+        description => 'Current state of the service process (systemd ActiveState).',
+    },
+    'desc' => {
+        type => 'string',
+        description => 'Description of the service.',
+    },
+    'name' => {
+        type => 'string',
+        description => 'Short identifier for the service (e.g., "pveproxy").',
+    },
+    'service' => {
+        type => 'string',
+        description => 'Systemd unit name (e.g., pveproxy).',
+    },
+    'state' => {
+        type => 'string',
+        # all systemd service unit substates
+        enum => [qw(
+            dead
+            condition
+            start-pre
+            start
+            start-post
+            running
+            exited
+            reload
+            reload-signal
+            reload-notify
+            mounting
+            stop
+            stop-watchdog
+            stop-sigterm
+            stop-sigkill
+            stop-post
+            final-watchdog
+            final-sigterm
+            final-sigkill
+            failed
+            dead-before-auto-restart
+            failed-before-auto-restart
+            dead-resources-pinned
+            auto-restart
+            auto-restart-queued
+            cleaning
+            unknown
+        )],
+        description => 'Execution status of the service (systemd SubState).',
+    },
+    'unit-state' => {
+        type => 'string',
+        enum => [qw(
+            enabled
+            enabled-runtime
+            linked
+            linked-runtime
+            alias
+            masked
+            masked-runtime
+            static
+            disabled
+            indirect
+            generated
+            transient
+            bad not-found
+            unknown
+        )],
+        description => 'Whether the service is enabled (systemd UnitFileState).',
+    },
+};
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -157,7 +233,7 @@ __PACKAGE__->register_method({
         type => 'array',
         items => {
             type => "object",
-            properties => {},
+            properties => $service_type,
         },
         links => [{ rel => 'child', href => "{service}" }],
     },
@@ -241,7 +317,7 @@ __PACKAGE__->register_method({
     },
     returns => {
         type => "object",
-        properties => {},
+        properties => $service_type,
     },
     code => sub {
         my ($param) = @_;
-- 
2.47.3


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

[pve-devel] [PATCH pve-manager v2 4/4] api: add replication config read return schema

[n.frey]

From: Nicolas Frey <n.frey@proxmox.com>

removed delete from schema for return types, as it is only used
in update/PUT.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 PVE/API2/ReplicationConfig.pm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/ReplicationConfig.pm b/PVE/API2/ReplicationConfig.pm
index 307ebe69..d6451d85 100644
--- a/PVE/API2/ReplicationConfig.pm
+++ b/PVE/API2/ReplicationConfig.pm
@@ -17,6 +17,10 @@ use PVE::API2::Replication;
 
 use base qw(PVE::RESTHandler);
 
+my $replication_config_return_type = PVE::ReplicationConfig->createSchema();
+# delete only used in update endpoint
+delete $replication_config_return_type->{delete};
+
 __PACKAGE__->register_method({
     name => 'index',
     path => '',
@@ -33,10 +37,7 @@ __PACKAGE__->register_method({
     },
     returns => {
         type => 'array',
-        items => {
-            type => "object",
-            properties => {},
-        },
+        items => $replication_config_return_type,
         links => [{ rel => 'child', href => "{id}" }],
     },
     code => sub {
@@ -75,7 +76,7 @@ __PACKAGE__->register_method({
             id => get_standard_option('pve-replication-id'),
         },
     },
-    returns => { type => 'object' },
+    returns => $replication_config_return_type,
     code => sub {
         my ($param) = @_;
 
-- 
2.47.3


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

[pve-devel] partially-applied: [PATCH pve-manager v2 0/4] api: add return schemas

[Thomas Lamprecht]

On Wed, 24 Sep 2025 13:59:00 +0200, n.frey@proxmox.com wrote:
> From: Nicolas Frey <n.frey@proxmox.com>
> 
> these four patches add the return schemas for endpoints in
> ACMEPlugin, APT, ReplicationConfig and Services using the
> createSchema() function where possible.
> Also tested functionality using pvesh.
> 
> [...]

Applied two of the four patches, will comment on the other two shortly, thanks!

Albeit I also squashed in a small change to the variable name to give them more
clarity, so that one reading the code can better tell what this is actually
used for, as just a _type prefix is rather opaque for these IMO.

[1/4] api: add ACME plugin return schema
      commit: 427e2627c0d19cf4907ae0157c84eef4a2d36764
[2/4] api: add APT versions return schema
      skipped for now
[3/4] api: add service state return schema
      commit: 35754e078debd8e5d1dffc1d67a291c31493289b
[4/4] api: add replication config read return schema
      skipped for now


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

Re: [pve-devel] [PATCH pve-manager v2 2/4] api: add APT versions return schema

[Thomas Lamprecht]

Am 24.09.25 um 13:59 schrieb n.frey@proxmox.com:
> From: Nicolas Frey <n.frey@proxmox.com>
> 
> listed only supported debian architectures [0] in enum for 'Arch'.
> listed all APT package states in enum for 'CurrentState'.

This is a bit specific and the sentence seems like it starts
somewhere in the middle, maybe this can be lead up to with
adding a little bit more context that avoids that one starts to
wonder what a APT version return schema has to do with Debian A+rchs.

> 
> [0] https://wiki.debian.org/SupportedArchitectures
> 
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
>  PVE/API2/APT.pm | 113 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 67 insertions(+), 46 deletions(-)
> 
> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
> index 0d07cf38..885b85ae 100644
> --- a/PVE/API2/APT.pm
> +++ b/PVE/API2/APT.pm

> @@ -769,6 +771,25 @@ __PACKAGE__->register_method({
>      },
>  });
>  
> +$apt_package_return_props->{CurrentState} = {
> +    type => 'string',
> +    description => 'Current state of the package installed on the system.',
> +    # Possible CurrentState variants according to AptPkg::Cache
> +    enum => [qw(Installed NotInstalled UnPacked HalfConfigured HalfInstalled ConfigFiles)],
> +};
> +
> +$apt_package_return_props->{RunningKernel} = {
> +    type => 'string',
> +    description => "Kernel release, only for package 'proxmox-ve'.",
> +    optional => 1,
> +};
> +
> +$apt_package_return_props->{ManagerVersion} = {
> +    type => 'string',
> +    description => "Version of the currently running pve-manager API server.",
> +    optional => 1,
> +};
> +

Modifying a hash reference that is already used above is rather dangerous, as just
some code line movement can introduce different behavior and wrong return types.

You rather should create a new (cloned) variable, e.g.:

my $versions_api_return_schema = {
    $apt_package_return_props->%*,
    CurrentState => {
        ...,
    },
    ...,
}

As this is then only used once you can also do that inline below, as
single-use intermediate variables are most often rather useless and just an
extra indirection. While they sometimes the can indeed help bring clarity,
here it would be pretty clear what's happening without that.

>  __PACKAGE__->register_method({
>      name => 'versions',
>      path => 'versions',
> @@ -788,7 +809,7 @@ __PACKAGE__->register_method({
>          type => "array",
>          items => {
>              type => "object",
> -            properties => {},
> +            properties => sort $apt_package_return_props,
>          },
>      },
>      code => sub {



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

Re: [pve-devel] [PATCH pve-manager v2 4/4] api: add replication config read return schema

[Thomas Lamprecht]

Am 24.09.25 um 13:59 schrieb n.frey@proxmox.com:
> From: Nicolas Frey <n.frey@proxmox.com>
> 
> removed delete from schema for return types, as it is only used
> in update/PUT.
> 
> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
>  PVE/API2/ReplicationConfig.pm | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/API2/ReplicationConfig.pm b/PVE/API2/ReplicationConfig.pm
> index 307ebe69..d6451d85 100644
> --- a/PVE/API2/ReplicationConfig.pm
> +++ b/PVE/API2/ReplicationConfig.pm
> @@ -17,6 +17,10 @@ use PVE::API2::Replication;
>  
>  use base qw(PVE::RESTHandler);
>  
> +my $replication_config_return_type = PVE::ReplicationConfig->createSchema();
> +# delete only used in update endpoint
> +delete $replication_config_return_type->{delete};
> +

Which makes one rather wonder why it gets returned for the create schema in
the first place – after all we do have a updateSchema

>  __PACKAGE__->register_method({
>      name => 'index',
>      path => '',
> @@ -33,10 +37,7 @@ __PACKAGE__->register_method({
>      },
>      returns => {
>          type => 'array',
> -        items => {
> -            type => "object",
> -            properties => {},
> -        },
> +        items => $replication_config_return_type,
>          links => [{ rel => 'child', href => "{id}" }],
>      },
>      code => sub {
> @@ -75,7 +76,7 @@ __PACKAGE__->register_method({
>              id => get_standard_option('pve-replication-id'),
>          },
>      },
> -    returns => { type => 'object' },
> +    returns => $replication_config_return_type,
>      code => sub {
>          my ($param) = @_;
>  



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

Re: [pve-devel] [PATCH pve-manager v2 2/4] api: add APT versions return schema

[Nicolas Frey]

On 10/2/25 10:50 AM, Thomas Lamprecht wrote:
> Am 24.09.25 um 13:59 schrieb n.frey@proxmox.com:
>> From: Nicolas Frey <n.frey@proxmox.com>
>>
>> listed only supported debian architectures [0] in enum for 'Arch'.
>> listed all APT package states in enum for 'CurrentState'.
> 
> This is a bit specific and the sentence seems like it starts
> somewhere in the middle, maybe this can be lead up to with
> adding a little bit more context that avoids that one starts to
> wonder what a APT version return schema has to do with Debian A+rchs.
> 

Will add context in v3, thanks!

>>
>> [0] https://wiki.debian.org/SupportedArchitectures
>>
>> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
>> ---
>>  PVE/API2/APT.pm | 113 ++++++++++++++++++++++++++++--------------------
>>  1 file changed, 67 insertions(+), 46 deletions(-)
>>
>> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
>> index 0d07cf38..885b85ae 100644
>> --- a/PVE/API2/APT.pm
>> +++ b/PVE/API2/APT.pm
> 
>> @@ -769,6 +771,25 @@ __PACKAGE__->register_method({
>>      },
>>  });
>>  
>> +$apt_package_return_props->{CurrentState} = {
>> +    type => 'string',
>> +    description => 'Current state of the package installed on the system.',
>> +    # Possible CurrentState variants according to AptPkg::Cache
>> +    enum => [qw(Installed NotInstalled UnPacked HalfConfigured HalfInstalled ConfigFiles)],
>> +};
>> +
>> +$apt_package_return_props->{RunningKernel} = {
>> +    type => 'string',
>> +    description => "Kernel release, only for package 'proxmox-ve'.",
>> +    optional => 1,
>> +};
>> +
>> +$apt_package_return_props->{ManagerVersion} = {
>> +    type => 'string',
>> +    description => "Version of the currently running pve-manager API server.",
>> +    optional => 1,
>> +};
>> +
> 
> Modifying a hash reference that is already used above is rather dangerous, as just
> some code line movement can introduce different behavior and wrong return types.
> 
> You rather should create a new (cloned) variable, e.g.:
> 
> my $versions_api_return_schema = {
>     $apt_package_return_props->%*,
>     CurrentState => {
>         ...,
>     },
>     ...,
> }
> 
> As this is then only used once you can also do that inline below, as
> single-use intermediate variables are most often rather useless and just an
> extra indirection. While they sometimes the can indeed help bring clarity,
> here it would be pretty clear what's happening without that.

Thanks for the feedback! I had a feeling modifying the hash reference might
not be the best approach (still getting used to Perl).
Will address in v3.

> 
>>  __PACKAGE__->register_method({
>>      name => 'versions',
>>      path => 'versions',
>> @@ -788,7 +809,7 @@ __PACKAGE__->register_method({
>>          type => "array",
>>          items => {
>>              type => "object",
>> -            properties => {},
>> +            properties => sort $apt_package_return_props,
>>          },
>>      },
>>      code => sub {
> 



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