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

[Fabian Grünbichler]

with downgrade of form_urlencoded version to packaged one folded in..

On May 23, 2024 1:18 pm, Gabriel Goller wrote:
> 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
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>