[pbs-devel] [PATCH proxmox{, -backup} v3 0/2] Remove influxdb bucket name restrictions

[pbs-devel] [PATCH proxmox{, -backup} v3 0/2] Remove influxdb bucket name restrictions

[Gabriel Goller]

Remove the regex for influxdb organizations and buckets. Influxdb does
not place any constraints on these names and allows all characters. This
also allows influxdb organization names with slashes. Some of these 
characters are not allowed in URI's, that's why we encode them in 
proxmox-metrics.

This also aligns the behavior to PVE as there are no restrictions there
either.

The motivation for this patch is this forum post:
https://forum.proxmox.com/threads/influx-db-organization-doesnt-allow-slash.145402/

v3, thanks @Fabian:
 - encode organization and bucket names

v2, thanks @Thomas:
 - include forum link

proxmox:

Gabriel Goller (1):
  metrics: encode influxdb org and bucket parameters

 proxmox-metrics/Cargo.toml           | 1 +
 proxmox-metrics/src/influxdb/http.rs | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)


proxmox-backup:

Gabriel Goller (1):
  api-types: remove influxdb bucket name restrictions

 pbs-api-types/src/metrics.rs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


Summary over all repositories:
  3 files changed, 9 insertions(+), 4 deletions(-)

-- 
Generated by git-murpp 0.5.0


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

[pbs-devel] [PATCH proxmox v3 1/2] metrics: encode influxdb org and bucket parameters

[Gabriel Goller]

In order to remove the current limitations on the bucket and
organization names, we need to make sure that they are transmitted
correctly. In order to do this, we encode them using the url crate.

This way we support organization/bucket names that include slashes,
whitespaces, etc.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-metrics/Cargo.toml           | 1 +
 proxmox-metrics/src/influxdb/http.rs | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/proxmox-metrics/Cargo.toml b/proxmox-metrics/Cargo.toml
index 98e3683..e97d55f 100644
--- a/proxmox-metrics/Cargo.toml
+++ b/proxmox-metrics/Cargo.toml
@@ -18,6 +18,7 @@ openssl.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 tokio = { workspace = true, features = [ "net", "sync" ] }
+url.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 b8c5c1e..8167b59 100644
--- a/proxmox-metrics/src/influxdb/http.rs
+++ b/proxmox-metrics/src/influxdb/http.rs
@@ -93,12 +93,17 @@ impl InfluxDbHttp {
             ""
         };
 
+        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!(
                 "{}/api/v2/write?org={}&bucket={}",
-                base_path, organization, bucket
+                base_path, encoded_org, encoded_bucket
             ))
             .build()?;
 
-- 
2.43.0



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

[pbs-devel] applied: [PATCH proxmox v3 1/2] metrics: encode influxdb org and bucket parameters

[Thomas Lamprecht]

Am 26/04/2024 um 16:02 schrieb Gabriel Goller:
> In order to remove the current limitations on the bucket and
> organization names, we need to make sure that they are transmitted
> correctly. In order to do this, we encode them using the url crate.
> 
> This way we support organization/bucket names that include slashes,
> whitespaces, etc.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  proxmox-metrics/Cargo.toml           | 1 +
>  proxmox-metrics/src/influxdb/http.rs | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

Having some tests here might be nice though. E.g., moving the URI assembly
into a separate method and having either a test module or doc-tests to
ensure that some common and odd cases, e.g. those requiring encoding, are
handled correctly.


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

Re: [pbs-devel] applied: [PATCH proxmox v3 1/2] metrics: encode influxdb org and bucket parameters

[Gabriel Goller]

On Fri Apr 26, 2024 at 5:52 PM CEST, Thomas Lamprecht wrote:
> Am 26/04/2024 um 16:02 schrieb Gabriel Goller:
> > In order to remove the current limitations on the bucket and
> > organization names, we need to make sure that they are transmitted
> > correctly. In order to do this, we encode them using the url crate.
> > 
> > This way we support organization/bucket names that include slashes,
> > whitespaces, etc.
> > 
> > Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> > ---
> >  proxmox-metrics/Cargo.toml           | 1 +
> >  proxmox-metrics/src/influxdb/http.rs | 7 ++++++-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> >
>
> applied, thanks!
>
> Having some tests here might be nice though. E.g., moving the URI assembly
> into a separate method and having either a test module or doc-tests to
> ensure that some common and odd cases, e.g. those requiring encoding, are
> handled correctly.

Sent a follow-up to the list!


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

[pbs-devel] [PATCH proxmox-backup v3 2/2] api-types: remove influxdb bucket name restrictions

[Gabriel Goller]

Remove the regex for influxdb organizations and buckets. Influxdb does
not place any constraints on these names and allows all characters. This
allows influxdb organization names with slashes.

Also remove a duplicate comment and add some missing ones.

This also aligns the behavior to PVE as there are no restrictions there
either.

The motivation for this patch is this forum post:
https://forum.proxmox.com/threads/influx-db-organization-doesnt-allow-slash.145402/

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-api-types/src/metrics.rs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/pbs-api-types/src/metrics.rs b/pbs-api-types/src/metrics.rs
index 6800c23b..23421035 100644
--- a/pbs-api-types/src/metrics.rs
+++ b/pbs-api-types/src/metrics.rs
@@ -12,14 +12,12 @@ pub const METRIC_SERVER_ID_SCHEMA: Schema = StringSchema::new("Metrics Server ID
     .schema();
 
 pub const INFLUXDB_BUCKET_SCHEMA: Schema = StringSchema::new("InfluxDB Bucket.")
-    .format(&PROXMOX_SAFE_ID_FORMAT)
     .min_length(3)
     .max_length(32)
     .default("proxmox")
     .schema();
 
 pub const INFLUXDB_ORGANIZATION_SCHEMA: Schema = StringSchema::new("InfluxDB Organization.")
-    .format(&PROXMOX_SAFE_ID_FORMAT)
     .min_length(3)
     .max_length(32)
     .default("proxmox")
@@ -129,13 +127,14 @@ pub struct InfluxDbHttp {
     pub enable: bool,
     /// The base url of the influxdb server
     pub url: String,
-    /// The Optional Token
     #[serde(skip_serializing_if = "Option::is_none")]
     /// The (optional) API token
     pub token: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    /// Named location where time series data is stored
     pub bucket: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    /// Workspace for a group of users
     pub organization: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     /// The (optional) maximum body size
-- 
2.43.0



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

[pbs-devel] applied: [PATCH proxmox-backup v3 2/2] api-types: remove influxdb bucket name restrictions

[Thomas Lamprecht]

Am 26/04/2024 um 16:02 schrieb Gabriel Goller:
> Remove the regex for influxdb organizations and buckets. Influxdb does
> not place any constraints on these names and allows all characters. This
> allows influxdb organization names with slashes.
> 
> Also remove a duplicate comment and add some missing ones.
> 
> This also aligns the behavior to PVE as there are no restrictions there
> either.
> 
> The motivation for this patch is this forum post:
> https://forum.proxmox.com/threads/influx-db-organization-doesnt-allow-slash.145402/
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pbs-api-types/src/metrics.rs | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
>

applied, thanks!


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