[pbs-devel] [PATCH v2] metrics: influxdb test uri creation

[Gabriel Goller]

Extract the URI creation for write and health URIs. Add unit test to
test the encoding of special characters in the organization and bucket
parameters.

Follow-up-to: bfa73aad ("metrics: encode influxdb org and bucket parameters")

Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
---

v2, thanks @Fabian:
 - rebased
 - import directly from form_urlencoded crate 
 - use serializer to generate whole query

 Cargo.toml                           |  1 +
 proxmox-metrics/Cargo.toml           |  2 +-
 proxmox-metrics/src/influxdb/http.rs | 94 ++++++++++++++++++++--------
 3 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 9746d0b4..946f3d31 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -64,6 +64,7 @@ crossbeam-channel = "0.5"
 endian_trait = "0.6"
 env_logger = "0.10"
 flate2 = "1.0"
+form_urlencoded = "1.2.1"
 futures = "0.3"
 handlebars = "3.0"
 hex = "0.4"
diff --git a/proxmox-metrics/Cargo.toml b/proxmox-metrics/Cargo.toml
index 937499ca..64fd3938 100644
--- a/proxmox-metrics/Cargo.toml
+++ b/proxmox-metrics/Cargo.toml
@@ -18,7 +18,7 @@ openssl.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 tokio = { workspace = true, features = [ "net", "sync" ] }
-url.workspace = true
+form_urlencoded.workspace = true
 
 proxmox-async.workspace = true
 proxmox-http = { workspace = true, features = [ "client" ] }
diff --git a/proxmox-metrics/src/influxdb/http.rs b/proxmox-metrics/src/influxdb/http.rs
index 7be68ad9..d773c16f 100644
--- a/proxmox-metrics/src/influxdb/http.rs
+++ b/proxmox-metrics/src/influxdb/http.rs
@@ -84,33 +84,7 @@ impl InfluxDbHttp {
             Client::with_ssl_connector(ssl_connector.build(), HttpOptions::default())
         };
 
-        let uri: http::uri::Uri = uri.parse()?;
-        let uri_parts = uri.into_parts();
-
-        let base_path = if let Some(ref p) = uri_parts.path_and_query {
-            p.path().trim_end_matches('/')
-        } else {
-            ""
-        };
-
-        let encoded_org: String =
-            url::form_urlencoded::byte_serialize(organization.as_bytes()).collect();
-        let encoded_bucket: String =
-            url::form_urlencoded::byte_serialize(bucket.as_bytes()).collect();
-
-        let writeuri = http::uri::Builder::new()
-            .scheme(uri_parts.scheme.clone().unwrap())
-            .authority(uri_parts.authority.clone().unwrap())
-            .path_and_query(format!(
-                "{base_path}/api/v2/write?org={encoded_org}&bucket={encoded_bucket}"
-            ))
-            .build()?;
-
-        let healthuri = http::uri::Builder::new()
-            .scheme(uri_parts.scheme.unwrap())
-            .authority(uri_parts.authority.unwrap())
-            .path_and_query(format!("{}/health", base_path))
-            .build()?;
+        let (writeuri, healthuri) = Self::create_uris(uri, organization, bucket)?;
 
         Ok(InfluxDbHttp {
             client,
@@ -123,6 +97,40 @@ impl InfluxDbHttp {
         })
     }
 
+    /// Return a tuple with the write_uri and the health_uri
+    fn create_uris(
+        uri: &str,
+        org: &str,
+        bucket: &str,
+    ) -> Result<(http::uri::Uri, http::uri::Uri), anyhow::Error> {
+        let uri: http::uri::Uri = uri.parse()?;
+        let uri_parts = uri.into_parts();
+
+        let base_path = if let Some(ref p) = uri_parts.path_and_query {
+            p.path().trim_end_matches('/')
+        } else {
+            ""
+        };
+
+        let write_uri_query: String = form_urlencoded::Serializer::new(String::new())
+            .append_pair("org", org)
+            .append_pair("bucket", bucket)
+            .finish();
+
+        Ok((
+            http::uri::Builder::new()
+                .scheme(uri_parts.scheme.clone().unwrap())
+                .authority(uri_parts.authority.clone().unwrap())
+                .path_and_query(format!("{base_path}/api/v2/write?{write_uri_query}"))
+                .build()?,
+            http::uri::Builder::new()
+                .scheme(uri_parts.scheme.unwrap())
+                .authority(uri_parts.authority.unwrap())
+                .path_and_query(format!("{base_path}/health"))
+                .build()?,
+        ))
+    }
+
     async fn test_connection(&self) -> Result<(), Error> {
         let mut request = http::Request::builder().method("GET").uri(&self.healthuri);
 
@@ -187,3 +195,35 @@ impl InfluxDbHttp {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::InfluxDbHttp;
+
+    #[test]
+    fn uri_encoding() {
+        let (writeuri, healthuri) =
+            InfluxDbHttp::create_uris("http://localhost/", "c ool/org", "a🔒nother&bu=ck?et")
+                .unwrap();
+        assert_eq!(writeuri.host(), Some("localhost"));
+        assert_eq!(writeuri.path(), "/api/v2/write");
+        assert_eq!(
+            writeuri.query(),
+            Some("org=c+ool%2Forg&bucket=a%F0%9F%94%92nother%26bu%3Dck%3Fet")
+        );
+
+        assert_eq!(healthuri.host(), Some("localhost"));
+        assert_eq!(healthuri.path(), "/health");
+        assert_eq!(healthuri.query(), None);
+
+        let (writeuri, healthuri) =
+            InfluxDbHttp::create_uris("http://localhost/", "org", "bucket").unwrap();
+        assert_eq!(writeuri.host(), Some("localhost"));
+        assert_eq!(writeuri.path(), "/api/v2/write");
+        assert_eq!(writeuri.query(), Some("org=org&bucket=bucket"));
+
+        assert_eq!(healthuri.host(), Some("localhost"));
+        assert_eq!(healthuri.path(), "/health");
+        assert_eq!(healthuri.query(), None);
+    }
+}
-- 
2.43.0