From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pbs-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 5FB981FF15C for <inbox@lore.proxmox.com>; Wed, 26 Mar 2025 15:44:51 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 31BDF3A3D6; Wed, 26 Mar 2025 15:44:46 +0100 (CET) From: Maximiliano Sandoval <m.sandoval@proxmox.com> To: pbs-devel@lists.proxmox.com Date: Wed, 26 Mar 2025 15:44:10 +0100 Message-Id: <20250326144410.430616-1-m.sandoval@proxmox.com> X-Mailer: git-send-email 2.39.5 MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.096 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: [pbs-devel] [PATCH proxmox] proxmox-client: add query builder X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion <pbs-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/> List-Post: <mailto:pbs-devel@lists.proxmox.com> List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com> A big proportion of the consumers of this API use the `format!` macro to define the base of the url, hence we use a `Cow<'_, str>` on the constructor to prevent unnecessary allocations. A `perl-api-path-builder` feature was added to satisfy the needs of pve-api-types. Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com> --- This is a follow-up of [1]. I will send the follow-ups separately. Differences from the RFC: - More docs - More tests - Moved into proxmox-client [1] https://lore.proxmox.com/pdm-devel/7qh5kchmrg4rlavkowqx44jiamcznarxn4konxww24zj7msuuv@gshqpr4ldxui/T/#u proxmox-client/Cargo.toml | 5 + proxmox-client/src/api_path_builder.rs | 242 +++++++++++++++++++++++++ proxmox-client/src/lib.rs | 4 + 3 files changed, 251 insertions(+) create mode 100644 proxmox-client/src/api_path_builder.rs diff --git a/proxmox-client/Cargo.toml b/proxmox-client/Cargo.toml index f890501e..027fbe3e 100644 --- a/proxmox-client/Cargo.toml +++ b/proxmox-client/Cargo.toml @@ -14,6 +14,7 @@ repository.workspace = true anyhow.workspace = true hex.workspace = true http.workspace = true +percent-encoding.workspace = true serde.workspace = true serde_json.workspace = true @@ -26,7 +27,11 @@ proxmox-login = { workspace = true, features = [ "http" ] } proxmox-http = { workspace = true, optional = true, features = [ "client" ] } hyper = { workspace = true, optional = true } +[dev-dependencies] +serde_plain.workspace = true + [features] default = [] hyper-client = [ "dep:openssl", "dep:hyper", "dep:proxmox-http", "dep:log" ] +perl-api-path-builder = [] webauthn = [ "proxmox-login/webauthn" ] diff --git a/proxmox-client/src/api_path_builder.rs b/proxmox-client/src/api_path_builder.rs new file mode 100644 index 00000000..bc2dfde4 --- /dev/null +++ b/proxmox-client/src/api_path_builder.rs @@ -0,0 +1,242 @@ +use std::borrow::Cow; + +/// Builder for API paths with a query. +/// +/// The [Self::arg] method can be used to add multiple arguments to the query. +/// +/// ```rust +/// use proxmox_client::ApiPathBuilder; +/// +/// let storage = "my-storage"; +/// let target = "my-target"; +/// let node = "pve01"; +/// let query = ApiPathBuilder::new(format!("/api2/extjs/nodes/{node}/storage")) +/// .arg("storage", storage) +/// .arg("target", target) +/// .build(); +/// +/// assert_eq!(&query, "/api2/extjs/nodes/pve01/storage?storage=my%2Dstorage&target=my%2Dtarget"); +/// ``` +/// +/// ## Compatibility with perl HTTP servers +/// +/// The following methods are added for compatibility with perl HTTP servers. +/// +/// - `bool_arg`: translates booleans to `"0"`/`"1"` +/// - `list_arg`: split lists so that they can be feed to perl's `split_list()` +/// +/// These methods require the `perl-api-path-builder` feature. +pub struct ApiPathBuilder { + url: String, + separator: char, +} + +impl ApiPathBuilder { + /// Creates a new builder from a base path. + pub fn new<'a>(base: impl Into<Cow<'a, str>>) -> Self { + Self { + url: base.into().into_owned(), + separator: '?', + } + } + + /// Adds an argument to the query. + /// + /// The name and value will be percent-encoded. + pub fn arg<T: std::fmt::Display>(mut self, name: &str, value: T) -> Self { + self.push_separator_and_name(name); + self.push_encoded(value.to_string().as_bytes()); + self + } + + /// Adds an optional argument to the query. + /// + /// Does nothing if the value is `None`. See [Self::arg] for more details. + pub fn maybe_arg<T: std::fmt::Display>(mut self, name: &str, value: &Option<T>) -> Self { + if let Some(value) = value { + self = self.arg(name, value); + } + self + } + + /// Builds the url. + pub fn build(self) -> String { + self.url + } + + fn push_separator_and_name(&mut self, name: &str) { + self.url.push(self.separator); + self.separator = '&'; + self.push_encoded(name.as_bytes()); + self.url.push('='); + } + + fn push_encoded(&mut self, value: &[u8]) { + let enc_value = percent_encoding::percent_encode(value, percent_encoding::NON_ALPHANUMERIC); + self.url.extend(enc_value); + } +} + +#[cfg(feature = "perl-api-path-builder")] +impl ApiPathBuilder { + /// Adds a boolean arg in a perl-friendly fashion. + /// + /// ```rust + /// use proxmox_client::ApiPathBuilder; + /// + /// let node = "pve01"; + /// let query = ApiPathBuilder::new(format!("/api2/extjs/nodes/{node}/storage")) + /// .bool_arg("enabled", true) + /// .build(); + /// + /// assert_eq!(&query, "/api2/extjs/nodes/pve01/storage?enabled=1"); + /// ``` + /// + /// `true` will be converted into `"1"` and `false` to `"0"`. + pub fn bool_arg(mut self, name: &str, value: bool) -> Self { + self.push_separator_and_name(name); + if value { + self.url.push('1'); + } else { + self.url.push('0'); + }; + self + } + + /// Adds an optional boolean arg in a perl-friendly fashion. + /// + /// Does nothing if `value` is `None`. See [Self::bool_arg] for more + /// details. + pub fn maybe_bool_arg(mut self, name: &str, value: Option<bool>) -> Self { + if let Some(value) = value { + self = self.bool_arg(name, value); + } + self + } + + /// Helper for building perl-friendly queries. + /// + /// For `<type>-list` entries we turn an array into a string ready for + /// perl's `split_list()` call. + /// + /// ```rust + /// use proxmox_client::api_path_builder::ApiPathBuilder; + /// + /// let content = vec!["backup", "images"]; + /// let node = "my_node"; + /// let query = &proxmox_client::ApiPathBuilder::new(format!("/api2/extjs/nodes/{node}/storage")) + /// .maybe_list_arg("content", &content) + /// .arg("type", "vm") + /// .build(); + /// + /// assert_eq!(&query, "/api2/extjs/nodes/my_node/storage?content=backup%00imagess"); + /// ``` + /// + /// The name and values will be percent-encoded. + pub fn list_arg<T: std::fmt::Display, I: IntoIterator<Item = T>>( + mut self, + name: &str, + values: I, + ) -> Self { + self.push_separator_and_name(name); + let mut list_separator = ""; + for entry in values.into_iter() { + self.url.push_str(list_separator); + list_separator = "%00"; + self.push_encoded(entry.to_string().as_bytes()); + } + self + } + + /// Helper for building perl-friendly queries. + /// + /// See [Self::list_arg] for more details. + pub fn maybe_list_arg<T: std::fmt::Display>( + mut self, + name: &str, + values: &Option<Vec<T>>, + ) -> Self { + if let Some(values) = values { + self = self.list_arg(name, values.iter()); + }; + self + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // See pdm_api_types::ConfigurationState. + #[derive(serde::Serialize)] + #[serde(rename_all = "kebab-case")] + enum ConfigurationState { + Active, + } + serde_plain::derive_display_from_serialize!(ConfigurationState); + + // See pve_api_types::ClusterResourceKind. + #[derive(serde::Serialize)] + enum ClusterResourceKind { + #[serde(rename = "vm")] + Vm, + } + serde_plain::derive_display_from_serialize!(ClusterResourceKind); + + #[test] + fn test_builder() { + let expected = "/api2/extjs/cluster/resources?type=vm"; + let ty = ClusterResourceKind::Vm; + + let query = ApiPathBuilder::new("/api2/extjs/cluster/resources") + .arg("type", ty) + .build(); + + assert_eq!(&query, expected); + + let second_expected = + "/api2/extjs/pve/remotes/some-remote/qemu/100/config?state=active&node=myNode"; + let state = ConfigurationState::Active; + let node = "myNode"; + let snapshot = None::<&str>; + + let second_query = + ApiPathBuilder::new("/api2/extjs/pve/remotes/some-remote/qemu/100/config") + .arg("state", state) + .arg("node", node) + .maybe_arg("snapshot", &snapshot) + .build(); + + assert_eq!(&second_query, &second_expected); + } +} + +#[cfg(all(test, feature = "perl-api-path-builder"))] +mod perl_tests { + use super::*; + + #[test] + fn test_perl_builder() { + let history = true; + let local_only = false; + let start_time = 1000; + + let expected_url = + "/api2/extjs/cluster/metrics/export?history=1&local%2Donly=0&start%2Dtime=1000"; + + let query = ApiPathBuilder::new("/api2/extjs/cluster/metrics/export") + .bool_arg("history", history) + .bool_arg("local-only", local_only) + .arg("start-time", start_time) + .build(); + assert_eq!(expected_url, query); + + let query_with_maybe = ApiPathBuilder::new("/api2/extjs/cluster/metrics/export") + .maybe_bool_arg("history", Some(history)) + .maybe_bool_arg("local-only", Some(local_only)) + .maybe_arg("start-time", &Some(start_time)) + .build(); + + assert_eq!(expected_url, query_with_maybe); + } +} diff --git a/proxmox-client/src/lib.rs b/proxmox-client/src/lib.rs index 00c48665..5978d501 100644 --- a/proxmox-client/src/lib.rs +++ b/proxmox-client/src/lib.rs @@ -14,6 +14,10 @@ pub use error::Error; pub use proxmox_login::tfa::TfaChallenge; pub use proxmox_login::{Authentication, Ticket}; +pub mod api_path_builder; + +pub use api_path_builder::ApiPathBuilder; + pub(crate) mod auth; pub use auth::{AuthenticationKind, Token}; -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel