* [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide
@ 2025-11-28 15:38 Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 1/4] tree-wide: add missing `returns` definitions to api macro Shannon Sterz
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-11-28 15:38 UTC (permalink / raw)
To: pdm-devel
these patches clean up the return section of all api endpoints as much
as possible. most of these are either additions of `returns` sections or
making the `returns` section match the actual api method it references.
the last two commit here are slightly different in that the first one
changes the actual return type of an api method. see the third commit
for the reasoning there. the last commit fixes up the description of an
api endpoint that was noticed along the way.
there are some other areas that seem inconsistent, but aren't addressed
here, as that would require more in-depth changes/discussion:
* `list_network_devices` in server/src/api/nodes/network.rs: this
endpoint has an returns sections that specifies a list of
`Interface`s, but the endpoint actually returns a `Value`. this
`Value` contains almost a list of `Interface`s, but extends them with
two field: `digest` and `iface`. ideally we could define these two
fields as optional field in `Interface` or similar.
* endpoints that upgrade to a websocket don't really have proper returns
sections, but i'm not sure what would be the proper definition anyway.
* pbs and pve tls probing endpoints: these return `TlsProbeOutcome` which
is an enum. this enum has a unit and an unnamed variant, which isn't
supported by the api macro. investigated a little bit, but there
doesn't seem to be a way to make this work without some refactoring.
* `refresh_remote_update_summaries` in server/src/api/remote_updates.rs:
this returns a parsed UPID struct instead of its string
representation. while this is fine, it is also inconsistent with all
other endpoints returning UPIDs afaict (though a lot of endpoints
return a `RemoteUpid`)
* `ping` in server/src/api/mod.rs: returns the string `"pong"` whereas
the equivalent pbs endpoint returns `{ "pong": true }`
the following endpoints could probably benefit from an access section:
* `get_node_rrddata` in server/src/nodes/rrddata.rs
* `list_remotes` in server/src/api/pbs/mod.rs
* `trigger_metric_collection` and `get_metric_collection_status` in
server/src/api/metric_collection.rs
Shannon Sterz (4):
tree-wide: add missing `returns` definitions to api macro
tree-wide: make `returns` defintion match the return type of api
methods
api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint
api: pbs: fix description of list namespace endpoint
server/src/api/access/tfa.rs | 10 ++++++
server/src/api/config/access/ad.rs | 1 -
server/src/api/config/access/ldap.rs | 1 -
server/src/api/config/access/openid.rs | 1 -
server/src/api/config/acme.rs | 9 +++++
server/src/api/config/views.rs | 1 +
server/src/api/metric_collection.rs | 10 +++++-
server/src/api/mod.rs | 22 ++++++++++++
server/src/api/nodes/certificates.rs | 9 +++++
server/src/api/nodes/network.rs | 3 ++
server/src/api/nodes/rrddata.rs | 7 ++++
server/src/api/nodes/tasks.rs | 50 +++++++++++++++++++++++++-
server/src/api/pbs/mod.rs | 9 ++++-
server/src/api/pbs/rrddata.rs | 14 ++++++--
server/src/api/pbs/tasks.rs | 8 +++--
server/src/api/pve/firewall.rs | 24 +++----------
server/src/api/pve/lxc.rs | 4 +--
server/src/api/pve/mod.rs | 8 +++++
server/src/api/pve/node.rs | 15 ++++++++
server/src/api/pve/qemu.rs | 1 +
server/src/api/pve/rrddata.rs | 32 +++++++++++++++--
server/src/api/pve/storage.rs | 2 +-
server/src/api/pve/tasks.rs | 8 ++++-
server/src/api/remote_tasks.rs | 8 +++++
server/src/api/remote_updates.rs | 3 ++
server/src/api/remotes.rs | 11 ++++--
server/src/api/resources.rs | 9 ++---
server/src/api/sdn/vnets.rs | 2 +-
server/src/api/sdn/zones.rs | 2 +-
29 files changed, 236 insertions(+), 48 deletions(-)
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 1/4] tree-wide: add missing `returns` definitions to api macro
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
@ 2025-11-28 15:38 ` Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 2/4] tree-wide: make `returns` defintion match the return type of api methods Shannon Sterz
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-11-28 15:38 UTC (permalink / raw)
To: pdm-devel
so this information shows up properly in a potential api viewer
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
server/src/api/access/tfa.rs | 10 ++++++
server/src/api/config/acme.rs | 9 +++++
server/src/api/config/views.rs | 1 +
server/src/api/metric_collection.rs | 10 +++++-
server/src/api/mod.rs | 22 ++++++++++++
server/src/api/nodes/certificates.rs | 9 +++++
server/src/api/nodes/network.rs | 3 ++
server/src/api/nodes/rrddata.rs | 7 ++++
server/src/api/nodes/tasks.rs | 50 +++++++++++++++++++++++++++-
server/src/api/pbs/mod.rs | 7 ++++
server/src/api/pbs/rrddata.rs | 14 ++++++--
server/src/api/pve/mod.rs | 8 +++++
server/src/api/pve/node.rs | 15 +++++++++
server/src/api/pve/qemu.rs | 1 +
server/src/api/pve/rrddata.rs | 21 ++++++++++++
server/src/api/remote_tasks.rs | 8 +++++
server/src/api/remote_updates.rs | 3 ++
server/src/api/remotes.rs | 7 ++++
server/src/api/resources.rs | 1 +
19 files changed, 202 insertions(+), 4 deletions(-)
diff --git a/server/src/api/access/tfa.rs b/server/src/api/access/tfa.rs
index a3bb628..8b1f7c6 100644
--- a/server/src/api/access/tfa.rs
+++ b/server/src/api/access/tfa.rs
@@ -103,6 +103,13 @@ fn list_tfa(rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<methods::TfaUser>, Er
&Permission::UserParam("userid"),
]),
},
+ returns: {
+ type: Array,
+ description: "A list of TFA entries for a user.",
+ items: {
+ type: methods::TypedTfaInfo,
+ }
+ }
)]
/// Add a TOTP secret to the user.
fn list_user_tfa(userid: Userid) -> Result<Vec<methods::TypedTfaInfo>, Error> {
@@ -125,6 +132,9 @@ fn list_user_tfa(userid: Userid) -> Result<Vec<methods::TypedTfaInfo>, Error> {
&Permission::UserParam("userid"),
]),
},
+ returns: {
+ type: methods::TypedTfaInfo,
+ }
)]
/// Get a single TFA entry.
fn get_tfa_entry(userid: Userid, id: String) -> Result<methods::TypedTfaInfo, Error> {
diff --git a/server/src/api/config/acme.rs b/server/src/api/config/acme.rs
index 2a5cc9d..0c583c4 100644
--- a/server/src/api/config/acme.rs
+++ b/server/src/api/config/acme.rs
@@ -111,6 +111,9 @@ pub fn list_accounts() -> Result<Vec<AccountEntry>, Error> {
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
},
protected: true,
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ },
)]
/// Register an ACME account.
fn register_account(
@@ -193,6 +196,9 @@ pub async fn get_account(name: AcmeAccountName) -> Result<AccountInfo, Error> {
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
},
protected: true,
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ },
)]
/// Update an ACME account.
pub fn update_account(
@@ -236,6 +242,9 @@ pub fn update_account(
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
},
protected: true,
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ },
)]
/// Deactivate an ACME account.
pub fn deactivate_account(
diff --git a/server/src/api/config/views.rs b/server/src/api/config/views.rs
index 79bc6f1..3e2865d 100644
--- a/server/src/api/config/views.rs
+++ b/server/src/api/config/views.rs
@@ -257,6 +257,7 @@ pub fn remove_view(id: String, digest: Option<ConfigDigest>) -> Result<(), Error
access: {
permission: &Permission::Privilege(&["view", "{id}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: { type: ViewConfig },
)]
/// Get the config of a single view.
pub fn read_view(id: String) -> Result<ViewConfig, Error> {
diff --git a/server/src/api/metric_collection.rs b/server/src/api/metric_collection.rs
index 0658fb1..b4c81c6 100644
--- a/server/src/api/metric_collection.rs
+++ b/server/src/api/metric_collection.rs
@@ -39,7 +39,15 @@ pub async fn trigger_metric_collection(remote: Option<String>) -> Result<(), Err
Ok(())
}
-#[api]
+#[api(
+ returns: {
+ type: Array,
+ description: "A list of metric collection statuses.",
+ items: {
+ type: MetricCollectionStatus,
+ }
+ }
+)]
/// Read metric collection status.
fn get_metric_collection_status() -> Result<Vec<MetricCollectionStatus>, Error> {
metric_collection::get_status()
diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs
index ef9964d..e3efed8 100644
--- a/server/src/api/mod.rs
+++ b/server/src/api/mod.rs
@@ -48,6 +48,10 @@ pub const ROUTER: Router = Router::new()
access: {
description: "Anyone can access this, just a cheap check if the API daemon is online.",
permission: &Permission::World,
+ },
+ returns: {
+ type: String,
+ description: "The string \"pong\"."
}
)]
/// A simple ping method. returns "pong"
@@ -59,6 +63,24 @@ fn ping() -> Result<String, Error> {
access: {
description: "Any valid user can access this.",
permission: &Permission::Anybody,
+ },
+ returns: {
+ type: Object,
+ description: "Version information.",
+ properties: {
+ version: {
+ type: String,
+ description: "The version string."
+ },
+ release: {
+ type: String,
+ description: "The package release.",
+ },
+ repoid: {
+ type: String,
+ description: "The repoid."
+ }
+ }
}
)]
/// Return the program's version/release info
diff --git a/server/src/api/nodes/certificates.rs b/server/src/api/nodes/certificates.rs
index 52c2ae3..5d8b924 100644
--- a/server/src/api/nodes/certificates.rs
+++ b/server/src/api/nodes/certificates.rs
@@ -201,6 +201,9 @@ pub async fn delete_custom_certificate() -> Result<(), Error> {
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
},
protected: true,
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ }
)]
/// Order a new ACME certificate.
pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error> {
@@ -223,6 +226,9 @@ pub fn new_acme_cert(force: bool, rpcenv: &mut dyn RpcEnvironment) -> Result<Str
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
},
protected: true,
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ }
)]
/// Renew the current ACME certificate if it expires within 30 days (or always if the `force`
/// parameter is set).
@@ -304,6 +310,9 @@ fn spawn_certificate_worker(
permission: &Permission::Privilege(&["system", "certificates"], PRIV_SYS_MODIFY, false),
},
protected: true,
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ }
)]
/// Renew the current ACME certificate if it expires within 30 days (or always if the `force`
/// parameter is set).
diff --git a/server/src/api/nodes/network.rs b/server/src/api/nodes/network.rs
index f5ad454..3fc9b0c 100644
--- a/server/src/api/nodes/network.rs
+++ b/server/src/api/nodes/network.rs
@@ -206,6 +206,9 @@ pub fn delete_interface(iface: String, digest: Option<ConfigDigest>) -> Result<(
access: {
permission: &Permission::Privilege(&["system", "network", "interfaces"], PRIV_SYS_MODIFY, false),
},
+ returns: {
+ schema: pdm_api_types::UPID_SCHEMA,
+ }
)]
/// Reload network configuration (requires ifupdown2).
pub async fn reload_network_config(rpcenv: &mut dyn RpcEnvironment) -> Result<String, Error> {
diff --git a/server/src/api/nodes/rrddata.rs b/server/src/api/nodes/rrddata.rs
index de7f5a5..7590096 100644
--- a/server/src/api/nodes/rrddata.rs
+++ b/server/src/api/nodes/rrddata.rs
@@ -38,6 +38,13 @@ impl DataPoint for PdmNodeDatapoint {
},
},
},
+ returns: {
+ type: Array,
+ description: "An array of RRD data points.",
+ items: {
+ type: PdmNodeDatapoint,
+ }
+ }
)]
/// Read RRD data for this PDM node.
fn get_node_rrddata(timeframe: RrdTimeframe, cf: RrdMode) -> Result<Vec<PdmNodeDatapoint>, Error> {
diff --git a/server/src/api/nodes/tasks.rs b/server/src/api/nodes/tasks.rs
index 9725f02..00dcf35 100644
--- a/server/src/api/nodes/tasks.rs
+++ b/server/src/api/nodes/tasks.rs
@@ -311,6 +311,39 @@ const TEST_STATUS_PARAM_SCHEMA: Schema = proxmox_schema::BooleanSchema::new(
)
.schema();
+#[sortable]
+const DATA_SCHEMA: Schema = proxmox_schema::ObjectSchema::new(
+ "A line of a task log.",
+ &sorted!([
+ (
+ "n",
+ false,
+ &proxmox_schema::IntegerSchema::new("Line number.")
+ .minimum(0)
+ .schema()
+ ),
+ (
+ "t",
+ false,
+ &proxmox_schema::StringSchema::new("The line of the log.").schema()
+ )
+ ]),
+)
+.schema();
+
+const TOTAL_SCHEMA: Schema = proxmox_schema::IntegerSchema::new("Total number of lines.")
+ .minimum(0)
+ .schema();
+
+const SUCCESS_SCHEMA: Schema =
+ proxmox_schema::IntegerSchema::new("Whether the request was successful.")
+ .minimum(0)
+ .maximum(1)
+ .schema();
+
+const ACTIVE_SCHEMA: Schema =
+ proxmox_schema::BooleanSchema::new("Whether the task is still active.").schema();
+
#[sortable]
pub const API_METHOD_READ_TASK_LOG: proxmox_router::ApiMethod = proxmox_router::ApiMethod::new(
&proxmox_router::ApiHandler::AsyncHttp(&read_task_log),
@@ -329,7 +362,22 @@ pub const API_METHOD_READ_TASK_LOG: proxmox_router::ApiMethod = proxmox_router::
.access(
Some("Users can access their own tasks, or need Sys.Audit on /system/tasks."),
&Permission::Anybody,
-);
+)
+.returns(proxmox_schema::ReturnType::new(
+ true,
+ &proxmox_schema::ObjectSchema::new(
+ "Returns lines of a task log. If `download` is set to `true`, instead of providing a JSON\
+ response the log file will be downloaded.",
+ &sorted!([
+ ("data", false, &DATA_SCHEMA),
+ ("total", false, &TOTAL_SCHEMA),
+ ("success", false, &SUCCESS_SCHEMA),
+ ("active", false, &ACTIVE_SCHEMA)
+ ]),
+ )
+ .schema(),
+));
+
fn read_task_log(
_parts: Parts,
_req_body: hyper::body::Incoming,
diff --git a/server/src/api/pbs/mod.rs b/server/src/api/pbs/mod.rs
index 15e4272..7679159 100644
--- a/server/src/api/pbs/mod.rs
+++ b/server/src/api/pbs/mod.rs
@@ -142,6 +142,11 @@ async fn list_datastores(remote: String) -> Result<Vec<pbs_api_types::DataStoreC
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "datastore", "{datastore}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: {
+ type: Array,
+ description: "A list of namespaces of a PBS remote's datastore.",
+ items: { type: pbs_api_types::NamespaceListItem }
+ }
)]
/// List the PBS remote's datastores.
async fn list_namespaces(
@@ -255,6 +260,7 @@ pub async fn connect_or_login(remote: &Remote) -> Result<Box<PbsClient>, Error>
permission:
&Permission::Privilege(&["/"], PRIV_SYS_MODIFY, false),
},
+ returns: { type: Remote }
)]
/// Scans the given connection info for pbs host information.
///
@@ -294,6 +300,7 @@ pub async fn scan_remote_pbs(
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
description: "The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}`."
},
+ returns: { type: pbs_api_types::NodeStatus }
)]
/// Get status for the PBS remote
async fn get_status(remote: String) -> Result<pbs_api_types::NodeStatus, Error> {
diff --git a/server/src/api/pbs/rrddata.rs b/server/src/api/pbs/rrddata.rs
index 74c670d..ffc8006 100644
--- a/server/src/api/pbs/rrddata.rs
+++ b/server/src/api/pbs/rrddata.rs
@@ -104,7 +104,12 @@ impl DataPoint for PbsDatastoreDataPoint {
access: {
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
description: "The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}`."
- }
+ },
+ returns: {
+ type: Array,
+ description: "A list of PBS node data points.",
+ items: { type: PbsNodeDataPoint },
+ },
)]
/// Read PBS node stats
async fn get_pbs_node_rrd_data(
@@ -133,7 +138,12 @@ async fn get_pbs_node_rrd_data(
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "datastore", "{datastore}"], PRIV_RESOURCE_AUDIT, false),
description: "The user needs to have at least the `Resource.Audit` privilege on `/resource/{remote}/datastore/{datastore}`."
- }
+ },
+ returns: {
+ type: Array,
+ description: "A list of PBS datastore data points.",
+ items: { type: PbsDatastoreDataPoint },
+ },
)]
/// Read PBS datastore stats
async fn get_pbs_datastore_rrd_data(
diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
index 34d9b76..05dd557 100644
--- a/server/src/api/pve/mod.rs
+++ b/server/src/api/pve/mod.rs
@@ -386,6 +386,7 @@ async fn probe_tls(
permission:
&Permission::Privilege(&["/"], PRIV_SYS_MODIFY, false),
},
+ returns: { type: Remote },
)]
/// Scans the given connection info for pve cluster information
///
@@ -474,6 +475,13 @@ pub async fn scan_remote_pve(
permission:
&Permission::Privilege(&["/"], PRIV_SYS_MODIFY, false),
},
+ returns: {
+ type: Array,
+ description: "A list of realms of a PVE remote.",
+ items: {
+ type: ListRealm,
+ }
+ },
)]
/// Scans the given connection info for pve cluster information
pub async fn list_realm_remote_pve(
diff --git a/server/src/api/pve/node.rs b/server/src/api/pve/node.rs
index 47bd5b3..d8690f6 100644
--- a/server/src/api/pve/node.rs
+++ b/server/src/api/pve/node.rs
@@ -48,6 +48,13 @@ const STORAGE_ROUTER: Router = Router::new()
},
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}"], PRIV_RESOURCE_AUDIT, false),
+ },
+ returns: {
+ type: Array,
+ description: "A list of network interfaces of a PVE remote.",
+ items: {
+ type: pve_api_types::NetworkInterface,
+ }
}
)]
/// Get network interfaces from PVE node
@@ -100,6 +107,13 @@ async fn get_network(
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}"], PRIV_RESOURCE_AUDIT, false),
description: "if `target` is set, also requires PRIV_RESOURCE_AUDIT on /resource/{remote}/node/{target}"
+ },
+ returns: {
+ type: Array,
+ description: "A list of storages of a PVE remote.",
+ items: {
+ type: pve_api_types::StorageInfo
+ }
}
)]
/// Get status for all datastores
@@ -130,6 +144,7 @@ async fn get_storages(
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: { type: pve_api_types::NodeStatus },
)]
/// Get status for the node
async fn get_status(remote: String, node: String) -> Result<pve_api_types::NodeStatus, Error> {
diff --git a/server/src/api/pve/qemu.rs b/server/src/api/pve/qemu.rs
index cd0d5d2..efab228 100644
--- a/server/src/api/pve/qemu.rs
+++ b/server/src/api/pve/qemu.rs
@@ -440,6 +440,7 @@ pub async fn qemu_migrate(
&Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_MIGRATE, false),
]),
},
+ returns: { type: QemuMigratePreconditions }
)]
/// Qemu (local) migrate preconditions
async fn qemu_migrate_preconditions(
diff --git a/server/src/api/pve/rrddata.rs b/server/src/api/pve/rrddata.rs
index 9fed967..226b675 100644
--- a/server/src/api/pve/rrddata.rs
+++ b/server/src/api/pve/rrddata.rs
@@ -180,6 +180,13 @@ impl DataPoint for PveStorageDataPoint {
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: {
+ type: Array,
+ description: "A list of RRD data points for a QEMU guest.",
+ items: {
+ type: QemuDataPoint
+ }
+ }
)]
/// Read qemu stats
async fn get_qemu_rrd_data(
@@ -209,6 +216,13 @@ async fn get_qemu_rrd_data(
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: {
+ type: Array,
+ description: "A list of RRD data points for an LXC guest.",
+ items: {
+ type: LxcDataPoint
+ }
+ }
)]
/// Read lxc stats
async fn get_lxc_rrd_data(
@@ -238,6 +252,13 @@ async fn get_lxc_rrd_data(
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: {
+ type: Array,
+ description: "A list of RRD data points for a PVE node.",
+ items: {
+ type: NodeDataPoint
+ }
+ }
)]
/// Read node stats
async fn get_node_rrd_data(
diff --git a/server/src/api/remote_tasks.rs b/server/src/api/remote_tasks.rs
index 02b6383..bce427f 100644
--- a/server/src/api/remote_tasks.rs
+++ b/server/src/api/remote_tasks.rs
@@ -49,6 +49,13 @@ const SUBDIRS: SubdirMap = &sorted!([
},
},
+ returns: {
+ type: Array,
+ description: "A list of tasks for all remotes.",
+ items: {
+ type: TaskListItem
+ }
+ },
)]
/// Get the list of tasks for all remotes.
async fn list_tasks(
@@ -91,6 +98,7 @@ async fn list_tasks(
},
},
},
+ returns: { type: TaskStatistics }
)]
/// Get task statistics for the specified filters.
async fn task_statistics(
diff --git a/server/src/api/remote_updates.rs b/server/src/api/remote_updates.rs
index bea2d86..f630e50 100644
--- a/server/src/api/remote_updates.rs
+++ b/server/src/api/remote_updates.rs
@@ -38,6 +38,7 @@ const SUBDIRS: SubdirMap = &sorted!([
permission: &Permission::Anybody,
description: "Resource.Modify privileges are needed on /resource/{remote}",
},
+ returns: { type: UpdateSummary }
)]
/// Return available update summary for managed remote nodes.
pub fn update_summary(rpcenv: &mut dyn RpcEnvironment) -> Result<UpdateSummary, Error> {
@@ -69,6 +70,7 @@ pub fn update_summary(rpcenv: &mut dyn RpcEnvironment) -> Result<UpdateSummary,
permission: &Permission::Anybody,
description: "Resource.Modify privileges are needed on /resource/{remote}",
},
+ returns: { type: UPID }
)]
/// Refresh the update summary of all remotes.
pub fn refresh_remote_update_summaries(rpcenv: &mut dyn RpcEnvironment) -> Result<UPID, Error> {
@@ -158,6 +160,7 @@ async fn apt_update_available(remote: String, node: String) -> Result<Vec<APTUpd
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}", "system"], PRIV_RESOURCE_MODIFY, false),
},
+ returns: { type: RemoteUpid }
)]
/// Update the APT database of a remote PVE node.
pub async fn apt_update_database(remote: String, node: String) -> Result<RemoteUpid, Error> {
diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs
index 5d5d9fd..bc0c871 100644
--- a/server/src/api/remotes.rs
+++ b/server/src/api/remotes.rs
@@ -376,6 +376,13 @@ impl DataPoint for RemoteDatapoint {
},
},
},
+ returns: {
+ type: Array,
+ description: "A list remote RRD data points.",
+ items: {
+ type: RemoteDatapoint,
+ }
+ }
)]
/// Read metric collection RRD data.
fn get_per_remote_rrd_data(
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 15c2c47..7eeb0ce 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -706,6 +706,7 @@ pub async fn get_subscription_status(
Only resources for which the user has `Resource.Audit` on `/resource/{remote_name}` will be
considered when calculating the top entities."
},
+ returns: { type: TopEntities }
)]
/// Returns the top X entities regarding the chosen type
async fn get_top_entities(
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 2/4] tree-wide: make `returns` defintion match the return type of api methods
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 1/4] tree-wide: add missing `returns` definitions to api macro Shannon Sterz
@ 2025-11-28 15:38 ` Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 3/4] api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint Shannon Sterz
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-11-28 15:38 UTC (permalink / raw)
To: pdm-devel
this takes usually one of the following forms:
- an array of the type is defined in the `returns` section, but the
type itself not a list of it is returned in the method.
- the `returns` section has a hand-crafted definition that is
incomplete, even though the type itself could be used directly.
- the type in the `returns` section does not match the actual return
type at all (e.g. the method returns a unit -> no `returns` section
should be specified)
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
server/src/api/config/access/ad.rs | 1 -
server/src/api/config/access/ldap.rs | 1 -
server/src/api/config/access/openid.rs | 1 -
server/src/api/pbs/tasks.rs | 8 ++++++--
server/src/api/pve/firewall.rs | 24 ++++--------------------
server/src/api/pve/lxc.rs | 4 ++--
server/src/api/pve/storage.rs | 2 +-
server/src/api/pve/tasks.rs | 8 +++++++-
server/src/api/remotes.rs | 4 +---
server/src/api/resources.rs | 8 +-------
server/src/api/sdn/vnets.rs | 2 +-
server/src/api/sdn/zones.rs | 2 +-
12 files changed, 24 insertions(+), 41 deletions(-)
diff --git a/server/src/api/config/access/ad.rs b/server/src/api/config/access/ad.rs
index 8167082..b5a3d5a 100644
--- a/server/src/api/config/access/ad.rs
+++ b/server/src/api/config/access/ad.rs
@@ -184,7 +184,6 @@ pub enum DeletableProperty {
},
},
},
- returns: { type: AdRealmConfig },
access: {
permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
},
diff --git a/server/src/api/config/access/ldap.rs b/server/src/api/config/access/ldap.rs
index c5e7732..374284f 100644
--- a/server/src/api/config/access/ldap.rs
+++ b/server/src/api/config/access/ldap.rs
@@ -216,7 +216,6 @@ pub enum DeletableProperty {
},
},
},
- returns: { type: LdapRealmConfig },
access: {
permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
},
diff --git a/server/src/api/config/access/openid.rs b/server/src/api/config/access/openid.rs
index 1832448..5e1764a 100644
--- a/server/src/api/config/access/openid.rs
+++ b/server/src/api/config/access/openid.rs
@@ -184,7 +184,6 @@ pub enum DeletableProperty {
},
},
},
- returns: { type: OpenIdRealmConfig },
access: {
permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
},
diff --git a/server/src/api/pbs/tasks.rs b/server/src/api/pbs/tasks.rs
index a178d81..7e86442 100644
--- a/server/src/api/pbs/tasks.rs
+++ b/server/src/api/pbs/tasks.rs
@@ -39,7 +39,11 @@ const UPID_API_SUBDIRS: SubdirMap = &sorted!([
// FIXME: fine-grained task filtering?
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
},
- returns: { type: pve_api_types::TaskStatus },
+ returns: {
+ type: Array,
+ description:"A list of tasks.",
+ items: { type: pbs_api_types::TaskListItem },
+ },
)]
/// Get the list of tasks either for a specific node, or query all at once.
async fn list_tasks(remote: String) -> Result<Vec<pbs_api_types::TaskListItem>, Error> {
@@ -89,7 +93,7 @@ async fn stop_task(remote: String, upid: RemoteUpid) -> Result<(), Error> {
// FIXME: fine-grained task filtering?
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
},
- returns: { type: pve_api_types::TaskStatus },
+ returns: { type: pdm_api_types::pbs::TaskStatus },
)]
/// Get the status of a task from a Proxmox VE instance.
pub async fn get_task_status(
diff --git a/server/src/api/pve/firewall.rs b/server/src/api/pve/firewall.rs
index 383b2d8..af11d58 100644
--- a/server/src/api/pve/firewall.rs
+++ b/server/src/api/pve/firewall.rs
@@ -337,11 +337,7 @@ pub async fn pve_firewall_status(
remote: { schema: REMOTE_ID_SCHEMA },
},
},
- returns: {
- type: Array,
- description: "Get firewall options.",
- items: { type: pve_api_types::ClusterFirewallOptions },
- },
+ returns: { type: pve_api_types::ClusterFirewallOptions },
access: {
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
},
@@ -456,11 +452,7 @@ pub async fn cluster_firewall_status(
},
},
},
- returns: {
- type: Array,
- description: "Get firewall options.",
- items: { type: pve_api_types::NodeFirewallOptions },
- },
+ returns: { type: pve_api_types::NodeFirewallOptions },
access: {
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
},
@@ -565,11 +557,7 @@ pub async fn cluster_firewall_rules(
vmid: { schema: VMID_SCHEMA },
},
},
- returns: {
- type: Array,
- description: "Get firewall options.",
- items: { type: pve_api_types::GuestFirewallOptions },
- },
+ returns: { type: pve_api_types::GuestFirewallOptions },
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
},
@@ -687,11 +675,7 @@ pub async fn node_firewall_rules(
vmid: { schema: VMID_SCHEMA },
},
},
- returns: {
- type: Array,
- description: "Get firewall options.",
- items: { type: pve_api_types::GuestFirewallOptions },
- },
+ returns: { type: pve_api_types::GuestFirewallOptions },
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
},
diff --git a/server/src/api/pve/lxc.rs b/server/src/api/pve/lxc.rs
index 96c0957..dc95783 100644
--- a/server/src/api/pve/lxc.rs
+++ b/server/src/api/pve/lxc.rs
@@ -61,7 +61,7 @@ const LXC_VM_SUBDIRS: SubdirMap = &sorted!([
returns: {
type: Array,
description: "Get a list of containers.",
- items: { type: pve_api_types::VmEntry },
+ items: { type: pve_api_types::LxcEntry },
},
access: {
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
@@ -194,7 +194,7 @@ pub async fn lxc_get_pending(
vmid: { schema: VMID_SCHEMA },
},
},
- returns: { type: pve_api_types::QemuStatus },
+ returns: { type: pve_api_types::LxcStatus },
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
},
diff --git a/server/src/api/pve/storage.rs b/server/src/api/pve/storage.rs
index a5338f4..6979711 100644
--- a/server/src/api/pve/storage.rs
+++ b/server/src/api/pve/storage.rs
@@ -27,7 +27,7 @@ const STORAGE_SUBDIR: SubdirMap = &sorted!([
storage: { schema: PVE_STORAGE_ID_SCHEMA, },
},
},
- returns: { type: pve_api_types::QemuStatus },
+ returns: { type: pve_api_types::StorageStatus },
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "storage", "{storage}"], PRIV_RESOURCE_AUDIT, false),
},
diff --git a/server/src/api/pve/tasks.rs b/server/src/api/pve/tasks.rs
index 5edce14..378f100 100644
--- a/server/src/api/pve/tasks.rs
+++ b/server/src/api/pve/tasks.rs
@@ -43,7 +43,13 @@ const UPID_API_SUBDIRS: SubdirMap = &sorted!([
// FIXME: fine-grained task filtering?
permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
},
- returns: { type: pve_api_types::TaskStatus },
+ returns: {
+ type: Array,
+ description: "A list of tasks.",
+ items: {
+ type: pve_api_types::ListTasksResponse
+ }
+ },
)]
/// Get the list of tasks either for a specific node, or query all at once.
async fn list_tasks(
diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs
index bc0c871..76b005d 100644
--- a/server/src/api/remotes.rs
+++ b/server/src/api/remotes.rs
@@ -67,9 +67,7 @@ pub fn get_remote<'a>(
description: "The list of configured remotes.",
type: Array,
items: {
- description: "Remote entry",
- type: Object,
- properties: {},
+ type: Remote,
},
},
)]
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 7eeb0ce..55056e1 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -458,13 +458,7 @@ pub(crate) async fn get_resources_impl(
},
}
},
- returns: {
- description: "Array of resources, grouped by remote",
- type: Array,
- items: {
- type: RemoteResources,
- }
- },
+ returns: { type: RemoteResources },
)]
/// Return the amount of configured/seen resources by type
pub async fn get_status(
diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index ebd2874..5e14c6a 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -149,7 +149,7 @@ async fn list_vnets(
},
},
},
- returns: { type: String, description: "Worker UPID" },
+ returns: { schema: pdm_api_types::UPID_SCHEMA },
)]
/// Create a VNet across multiple remotes
async fn create_vnet(
diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index 2ae3d67..c455279 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -158,7 +158,7 @@ pub async fn list_zones(
},
},
},
- returns: { type: String, description: "Worker UPID" },
+ returns: { schema: pdm_api_types::UPID_SCHEMA },
)]
/// Create a zone across multiple remotes
async fn create_zone(
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 3/4] api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 1/4] tree-wide: add missing `returns` definitions to api macro Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 2/4] tree-wide: make `returns` defintion match the return type of api methods Shannon Sterz
@ 2025-11-28 15:38 ` Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 4/4] api: pbs: fix description of list namespace endpoint Shannon Sterz
2025-11-28 18:53 ` [pdm-devel] applied: [PATCH datacenter-manager 0/4] clean up return statements tree wide Thomas Lamprecht
4 siblings, 0 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-11-28 15:38 UTC (permalink / raw)
To: pdm-devel
this worked so far by accident as `NodeDataPoint` has the field that
are also specified in `PveStorageDataPoint`, but marks all additional
fields as optional. fix this by using the proper return type.
pdm-client already uses the correct return type anyway.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
server/src/api/pve/rrddata.rs | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/server/src/api/pve/rrddata.rs b/server/src/api/pve/rrddata.rs
index 226b675..08db107 100644
--- a/server/src/api/pve/rrddata.rs
+++ b/server/src/api/pve/rrddata.rs
@@ -289,8 +289,15 @@ async fn get_node_rrd_data(
access: {
permission: &Permission::Privilege(&["resource", "{remote}", "storage", "{storage}"], PRIV_RESOURCE_AUDIT, false),
},
+ returns: {
+ type: Array,
+ description:"A list of RRD data points on a PVE remote's storage.",
+ items: {
+ type: PveStorageDataPoint,
+ }
+ }
)]
-/// Read node stats
+/// Read storage stats
async fn get_storage_rrd_data(
remote: String,
node: String,
@@ -298,7 +305,7 @@ async fn get_storage_rrd_data(
timeframe: RrdTimeframe,
cf: RrdMode,
_param: Value,
-) -> Result<Vec<NodeDataPoint>, Error> {
+) -> Result<Vec<PveStorageDataPoint>, Error> {
let base = format!("pve/{remote}/storage/{node}/{storage}");
rrd_common::get_rrd_datapoints(remote, base, timeframe, cf).await
}
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pdm-devel] [PATCH datacenter-manager 4/4] api: pbs: fix description of list namespace endpoint
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
` (2 preceding siblings ...)
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 3/4] api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint Shannon Sterz
@ 2025-11-28 15:38 ` Shannon Sterz
2025-11-28 18:53 ` [pdm-devel] applied: [PATCH datacenter-manager 0/4] clean up return statements tree wide Thomas Lamprecht
4 siblings, 0 replies; 6+ messages in thread
From: Shannon Sterz @ 2025-11-28 15:38 UTC (permalink / raw)
To: pdm-devel
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
server/src/api/pbs/mod.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/src/api/pbs/mod.rs b/server/src/api/pbs/mod.rs
index 7679159..badefc3 100644
--- a/server/src/api/pbs/mod.rs
+++ b/server/src/api/pbs/mod.rs
@@ -148,7 +148,7 @@ async fn list_datastores(remote: String) -> Result<Vec<pbs_api_types::DataStoreC
items: { type: pbs_api_types::NamespaceListItem }
}
)]
-/// List the PBS remote's datastores.
+/// List the namespaces of PBS remote's datastore.
async fn list_namespaces(
remote: String,
params: pbs_client::DatstoreListNamespaces,
--
2.47.3
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pdm-devel] applied: [PATCH datacenter-manager 0/4] clean up return statements tree wide
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
` (3 preceding siblings ...)
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 4/4] api: pbs: fix description of list namespace endpoint Shannon Sterz
@ 2025-11-28 18:53 ` Thomas Lamprecht
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2025-11-28 18:53 UTC (permalink / raw)
To: pdm-devel, Shannon Sterz
On Fri, 28 Nov 2025 16:38:51 +0100, Shannon Sterz wrote:
> these patches clean up the return section of all api endpoints as much
> as possible. most of these are either additions of `returns` sections or
> making the `returns` section match the actual api method it references.
>
> the last two commit here are slightly different in that the first one
> changes the actual return type of an api method. see the third commit
> for the reasoning there. the last commit fixes up the description of an
> api endpoint that was noticed along the way.
>
> [...]
Applied, thanks!
[1/4] tree-wide: add missing `returns` definitions to api macro
commit: fa143bfa6940401c594b993428c38cc5254c83e8
[2/4] tree-wide: make `returns` defintion match the return type of api methods
commit: 1e43329753864f173673fcb5b50106c5e875e919
[3/4] api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint
commit: bd76550329c14cc8011da06ff45906a7ef66011c
[4/4] api: pbs: fix description of list namespace endpoint
commit: 7600289cb63b0b980df64024eab6960df18d57fd
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-28 18:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 1/4] tree-wide: add missing `returns` definitions to api macro Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 2/4] tree-wide: make `returns` defintion match the return type of api methods Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 3/4] api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 4/4] api: pbs: fix description of list namespace endpoint Shannon Sterz
2025-11-28 18:53 ` [pdm-devel] applied: [PATCH datacenter-manager 0/4] clean up return statements tree wide Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox