[pbs-devel] [PATCH proxmox{, -backup} 00/10] s3: implement list buckets and use bucket selector for datastore creation

[pbs-devel] [PATCH proxmox{, -backup} 00/10] s3: implement list buckets and use bucket selector for datastore creation

[Christian Ebner]

This patches aim to increase usability for the user by allowing to list
accessible buckets during datastore creation.

For this, the proxmox s3 client is extended by the list buckets method, fetching
accessible buckets from the s3 api. This requires the client instantiation to be
possible without the bucket name, making it therefore optional. Further, the
regex for the config endpoint url must be restricted to have the bucket name
template pattern always at the start of the template. With these changes, it is
then possible to fetch the buckets given the required permissions to do so.

On the proxmox backup server side, the api endpoints to fetch the bucket list
are implemented as well as the corresponding cli command. Finally, the bucket
name field in the datastore edit window is replaced by an s3 bucket selector,
which loads the accessible bucket names via the api.

proxmox:

Christian Ebner (4):
  s3 client: restrict bucket template pattern to start of endpoint url
  s3 client: make bucket name optional in S3 client options
  s3 client: implement list buckets method
  s3 client: api types: add bucket list item type

 proxmox-s3-client/examples/s3_client.rs  |  2 +-
 proxmox-s3-client/src/api_types.rs       | 17 ++++++-
 proxmox-s3-client/src/client.rs          | 39 +++++++++++++---
 proxmox-s3-client/src/response_reader.rs | 58 ++++++++++++++++++++++++
 4 files changed, 108 insertions(+), 8 deletions(-)


proxmox-backup:

Christian Ebner (6):
  ui: fix formatting issues using proxmox-biome
  datastore: wrap bucket name, as in is now optional in the s3 client
  api: admin s3: make store prefix for check command optional
  api: config s3: add bucket list api endpoint
  bin: expose list buckets as proxmox-backup-manager s3 subcommand
  ui: replace bucket field by bucket selector

 pbs-datastore/src/datastore.rs       |  2 +-
 src/api2/admin/s3.rs                 |  6 ++--
 src/api2/config/datastore.rs         |  4 +--
 src/api2/config/s3.rs                | 48 +++++++++++++++++++++++--
 src/bin/proxmox_backup_manager/s3.rs | 28 +++++++++++++--
 www/Makefile                         |  1 +
 www/Utils.js                         |  5 ++-
 www/form/S3BucketSelector.js         | 52 ++++++++++++++++++++++++++++
 www/window/DataStoreEdit.js          | 27 +++++++++++----
 9 files changed, 155 insertions(+), 18 deletions(-)
 create mode 100644 www/form/S3BucketSelector.js


Summary over all repositories:
  13 files changed, 263 insertions(+), 26 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pbs-devel] [PATCH proxmox 1/4] s3 client: restrict bucket template pattern to start of endpoint url

[Christian Ebner]

The template patterns were introduced with the goal to increase
reusability of the same endpoint configuration for multiple buckets.
The pattern are then replaced with the respective configuration value
by the client on instantiation.

However, for some operations one might not know a bucket name
a-priori and the bucket name might not be required. For example, a
list buckets operations can be performed on the endpoint url without
the bucket name subdomain on the S3 API endpoint.

Therefore, restrict the bucket pattern to be at the start of the
endpoint name, allowing to strip it if not provided, and perform the
requests without the subdomain prefix in that case.

This is a breaking change.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-s3-client/src/api_types.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-s3-client/src/api_types.rs b/proxmox-s3-client/src/api_types.rs
index eff15f37..4a589717 100644
--- a/proxmox-s3-client/src/api_types.rs
+++ b/proxmox-s3-client/src/api_types.rs
@@ -11,7 +11,7 @@ use proxmox_schema::{api, const_regex, ApiStringFormat, Schema, StringSchema, Up
 /// Regex to match S3 endpoint full qualified domain names, including template patterns for bucket
 /// name or region.
 pub const S3_ENDPOINT_NAME_STR: &str = concatcp!(
-    r"(?:(?:(", DNS_LABEL_STR, r"|\{\{bucket\}\}|\{\{region\}\})\.)*", DNS_LABEL_STR, ")"
+    r"(^\{\{bucket\}\}\.)*(?:(?:(", DNS_LABEL_STR, r"|\{\{region\}\})\.)*", DNS_LABEL_STR, ")"
 );
 
 const_regex! {
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox 2/4] s3 client: make bucket name optional in S3 client options

[Christian Ebner]

Not all operations require the bucket name, e.g. the list buckets
operation works also without the bucket name.

Since vhost style addressing might contain the bucket name as
subdomain template pattern, strip the pattern in that case and use
the url without the subdomain.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-s3-client/examples/s3_client.rs |  2 +-
 proxmox-s3-client/src/client.rs         | 25 ++++++++++++++++++++-----
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/proxmox-s3-client/examples/s3_client.rs b/proxmox-s3-client/examples/s3_client.rs
index 1cbb3939..4a9625cb 100644
--- a/proxmox-s3-client/examples/s3_client.rs
+++ b/proxmox-s3-client/examples/s3_client.rs
@@ -28,7 +28,7 @@ async fn run() -> Result<(), anyhow::Error> {
         // Must match the port the api is listening on
         port: Some(7480),
         // Name of the bucket to be used
-        bucket: "testbucket".to_string(),
+        bucket: Some("testbucket".to_string()),
         common_prefix: "teststore".to_string(),
         path_style: false,
         access_key: "<your-access-key>".to_string(),
diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
index 3a981bf4..91188f1a 100644
--- a/proxmox-s3-client/src/client.rs
+++ b/proxmox-s3-client/src/client.rs
@@ -54,7 +54,7 @@ pub struct S3ClientOptions {
     /// Port to access S3 object store.
     pub port: Option<u16>,
     /// Bucket to access S3 object store.
-    pub bucket: String,
+    pub bucket: Option<String>,
     /// Common prefix within bucket to use for objects keys for this client instance.
     pub common_prefix: String,
     /// Use path style bucket addressing over vhost style.
@@ -76,7 +76,7 @@ impl S3ClientOptions {
     pub fn from_config(
         config: S3ClientConfig,
         secret_key: String,
-        bucket: String,
+        bucket: Option<String>,
         common_prefix: String,
     ) -> Self {
         Self {
@@ -154,9 +154,16 @@ impl S3Client {
         } else {
             options.endpoint.clone()
         };
+
         let authority = authority_template
-            .replace("{{bucket}}", &options.bucket)
             .replace("{{region}}", &options.region);
+
+        let authority = if let Some(bucket) = &options.bucket {
+            authority.replace("{{bucket}}", bucket)
+        } else {
+            authority.replace("{{bucket}}.", "")
+        };
+
         let authority = Authority::try_from(authority)?;
 
         let put_rate_limiter = options.put_rate_limit.map(|limit| {
@@ -452,8 +459,12 @@ impl S3Client {
         source_key: S3ObjectKey,
         destination_key: S3ObjectKey,
     ) -> Result<CopyObjectResponse, Error> {
+        let bucket = match &self.options.bucket {
+            Some(bucket) => bucket,
+            None => bail!("missing bucket name for copy source"),
+        };
         let copy_source =
-            source_key.to_copy_source_key(&self.options.bucket, &self.options.common_prefix);
+            source_key.to_copy_source_key(bucket, &self.options.common_prefix);
         let copy_source = aws_sign_v4_uri_encode(&copy_source, true);
         let destination_key = destination_key.to_full_key(&self.options.common_prefix);
         let destination_key = aws_sign_v4_uri_encode(&destination_key, true);
@@ -652,7 +663,11 @@ impl S3Client {
         }
         let path = aws_sign_v4_uri_encode(path, true);
         let mut path_and_query = if self.options.path_style {
-            format!("/{bucket}/{path}", bucket = self.options.bucket)
+            if let Some(bucket) = &self.options.bucket {
+                format!("/{bucket}/{path}")
+            } else {
+                format!("/{path}")
+            }
         } else {
             format!("/{path}")
         };
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox 3/4] s3 client: implement list buckets method

[Christian Ebner]

Extends the client by the list buckets method which allows to
fetch the buckets owned by the user with who's access key the
request is signed.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-s3-client/src/client.rs          | 14 +++++-
 proxmox-s3-client/src/response_reader.rs | 58 ++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
index 91188f1a..a2c62811 100644
--- a/proxmox-s3-client/src/client.rs
+++ b/proxmox-s3-client/src/client.rs
@@ -28,7 +28,7 @@ use crate::aws_sign_v4::{aws_sign_v4_signature, aws_sign_v4_uri_encode};
 use crate::object_key::S3ObjectKey;
 use crate::response_reader::{
     CopyObjectResponse, DeleteObjectsResponse, GetObjectResponse, HeadObjectResponse,
-    ListObjectsV2Response, PutObjectResponse, ResponseReader,
+    ListBucketsResponse, ListObjectsV2Response, PutObjectResponse, ResponseReader,
 };
 
 const S3_HTTP_CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
@@ -319,6 +319,18 @@ impl S3Client {
         Ok(())
     }
 
+    /// List all buckets owned by the user authenticated via the access key.
+    /// See reference docs: https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html
+    pub async fn list_buckets(&self) -> Result<ListBucketsResponse, Error> {
+        let request = Request::builder()
+            .method(Method::GET)
+            .uri(self.build_uri("/", &[])?)
+            .body(Body::empty())?;
+        let response = self.send(request).await?;
+        let response_reader = ResponseReader::new(response);
+        response_reader.list_buckets_response().await
+    }
+
     /// Fetch metadata from an object without returning the object itself.
     /// See reference docs: https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html
     pub async fn head_object(
diff --git a/proxmox-s3-client/src/response_reader.rs b/proxmox-s3-client/src/response_reader.rs
index a7c71639..4514eaa8 100644
--- a/proxmox-s3-client/src/response_reader.rs
+++ b/proxmox-s3-client/src/response_reader.rs
@@ -153,6 +153,38 @@ pub struct CopyObjectResult {
     pub last_modified: LastModifiedTimestamp,
 }
 
+/// Subset of the list buckets response
+/// https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html#API_ListBuckets_ResponseElements
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct ListBucketsResponse {
+    pub buckets: Vec<Bucket>,
+}
+/// Subset of the list buckets response
+/// https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html#API_ListBuckets_ResponseElements
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct ListAllMyBucketsResult {
+    pub buckets: Option<Vec<Buckets>>,
+}
+
+/// Subset used to deserialize the list buckets response
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct Buckets {
+    bucket: Bucket,
+}
+
+/// Subset used to deserialize the list buckets response
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+pub struct Bucket {
+    pub name: String,
+    pub bucket_arn: Option<String>,
+    pub bucket_region: Option<String>,
+    pub creation_date: LastModifiedTimestamp,
+}
+
 impl ResponseReader {
     pub(crate) fn new(response: Response<Incoming>) -> Self {
         Self { response }
@@ -339,6 +371,32 @@ impl ResponseReader {
         })
     }
 
+    pub(crate) async fn list_buckets_response(self) -> Result<ListBucketsResponse, Error> {
+        let (parts, body) = self.response.into_parts();
+        let body = body.collect().await?.to_bytes();
+
+        match parts.status {
+            StatusCode::OK => (),
+            status_code => {
+                Self::log_error_response_utf8(body);
+                bail!("unexpected status code {status_code}")
+            }
+        };
+
+        let body = String::from_utf8(body.to_vec())?;
+
+        let list_buckets_result: ListAllMyBucketsResult =
+            serde_xml_rs::from_str(&body).context("failed to parse response body")?;
+
+        let buckets = list_buckets_result
+            .buckets
+            .unwrap_or_default()
+            .into_iter()
+            .map(|bucket| bucket.bucket)
+            .collect();
+        Ok(ListBucketsResponse { buckets })
+    }
+
     fn log_error_response_utf8(body: Bytes) {
         if let Ok(body) = String::from_utf8(body.to_vec()) {
             if !body.is_empty() {
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox 4/4] s3 client: api types: add bucket list item type

[Christian Ebner]

Define a dedicated API type to be used in bucket list responses
from the PBS api.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-s3-client/src/api_types.rs | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/proxmox-s3-client/src/api_types.rs b/proxmox-s3-client/src/api_types.rs
index 4a589717..265fedb9 100644
--- a/proxmox-s3-client/src/api_types.rs
+++ b/proxmox-s3-client/src/api_types.rs
@@ -191,3 +191,18 @@ pub struct S3ClientConfigWithoutSecret {
     #[serde(flatten)]
     pub config: S3ClientConfig,
 }
+
+#[api(
+    properties: {
+        name: {
+            schema: S3_BUCKET_NAME_SCHEMA,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize, Clone, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// S3 bucket list item.
+pub struct S3BucketListItem {
+    /// S3 bucket name.
+    pub name: String,
+}
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox-backup 1/6] ui: fix formatting issues using proxmox-biome

[Christian Ebner]

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/Utils.js                | 5 ++++-
 www/window/DataStoreEdit.js | 5 +++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/www/Utils.js b/www/Utils.js
index ccdae522c..a80a59a5b 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -443,7 +443,10 @@ Ext.define('PBS.Utils', {
                 PBS.Utils.render_drive_load_media_id(id, gettext('Load Media')),
             logrotate: [null, gettext('Log Rotation')],
             'mount-device': [gettext('Datastore'), gettext('Mount Device')],
-            'mount-sync-jobs': [gettext('Datastore'), gettext('sync jobs handler triggered by mount')],
+            'mount-sync-jobs': [
+                gettext('Datastore'),
+                gettext('sync jobs handler triggered by mount'),
+            ],
             prune: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Prune')),
             prunejob: (type, id) => PBS.Utils.render_prune_job_worker_id(id, gettext('Prune Job')),
             reader: (type, id) => PBS.Utils.render_datastore_worker_id(id, gettext('Read Objects')),
diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
index b414bd4c0..8296e0b04 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -78,8 +78,9 @@ Ext.define('PBS.DataStoreEdit', {
                                 let s3ClientSelector = inputPanel.down('[name=s3client]');
                                 let overwriteInUseField =
                                     inputPanel.down('[name=overwrite-in-use]');
-                                let reuseDatastore =
-                                    inputPanel.down('[name=reuse-datastore]').getValue();
+                                let reuseDatastore = inputPanel
+                                    .down('[name=reuse-datastore]')
+                                    .getValue();
 
                                 uuidEditField.setDisabled(!isRemovable);
                                 uuidEditField.allowBlank = !isRemovable;
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox-backup 2/6] datastore: wrap bucket name, as in is now optional in the s3 client

[Christian Ebner]

The bucket name is now optional so it is possible to also instantiate
a client to perform operations without a bucket, e.g. listing of
available buckets. Adapt the callsides accordingly.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 2 +-
 src/api2/admin/s3.rs           | 2 +-
 src/api2/config/datastore.rs   | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index e8be576f7..01122c6de 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -256,7 +256,7 @@ impl DataStore {
                 let options = S3ClientOptions::from_config(
                     config.config,
                     config.secret_key,
-                    bucket,
+                    Some(bucket),
                     self.name().to_owned(),
                 );
                 let s3_client = S3Client::new(options)?;
diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
index 1a0932945..90cc41ea6 100644
--- a/src/api2/admin/s3.rs
+++ b/src/api2/admin/s3.rs
@@ -48,7 +48,7 @@ pub async fn check(
         .context("config lookup failed")?;
 
     let options =
-        S3ClientOptions::from_config(config.config, config.secret_key, bucket, store_prefix);
+        S3ClientOptions::from_config(config.config, config.secret_key, Some(bucket), store_prefix);
 
     let test_object_key =
         S3ObjectKey::try_from(".s3-client-test").context("failed to generate s3 object key")?;
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index d24323152..cf8e306cc 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -151,7 +151,7 @@ pub(crate) fn do_create_datastore(
         let options = S3ClientOptions::from_config(
             config.config,
             config.secret_key,
-            bucket,
+            Some(bucket),
             datastore.name.to_owned(),
         );
         let s3_client = S3Client::new(options).context("failed to create s3 client")?;
@@ -379,7 +379,7 @@ pub fn create_datastore(
             let options = S3ClientOptions::from_config(
                 config.config,
                 config.secret_key,
-                bucket,
+                Some(bucket),
                 store_name.clone(),
             );
             let s3_client = S3Client::new(options).context("failed to create s3 client")?;
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox-backup 3/6] api: admin s3: make store prefix for check command optional

[Christian Ebner]

The store prefix is not really required for basic checks and was non
optional since the s3 clients relies on it for object key expansion,
so it should not be forgotten on regular datastore operations.

So make it optional instead. Since this relaxes the api constraints,
it is not a breaking api change.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/s3.rs                 | 4 +++-
 src/bin/proxmox_backup_manager/s3.rs | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
index 90cc41ea6..209feaebc 100644
--- a/src/api2/admin/s3.rs
+++ b/src/api2/admin/s3.rs
@@ -28,6 +28,7 @@ use pbs_config::s3::S3_CFG_TYPE_ID;
             "store-prefix": {
                 type: String,
                 description: "Store prefix within bucket for S3 object keys (commonly datastore name)",
+                optional: true,
             },
         },
     },
@@ -39,7 +40,7 @@ use pbs_config::s3::S3_CFG_TYPE_ID;
 pub async fn check(
     s3_client_id: String,
     bucket: String,
-    store_prefix: String,
+    store_prefix: Option<String>,
     _rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
     let (config, _digest) = pbs_config::s3::config()?;
@@ -47,6 +48,7 @@ pub async fn check(
         .lookup(S3_CFG_TYPE_ID, &s3_client_id)
         .context("config lookup failed")?;
 
+    let store_prefix = store_prefix.unwrap_or_default();
     let options =
         S3ClientOptions::from_config(config.config, config.secret_key, Some(bucket), store_prefix);
 
diff --git a/src/bin/proxmox_backup_manager/s3.rs b/src/bin/proxmox_backup_manager/s3.rs
index 867bc8e6b..f660baf4a 100644
--- a/src/bin/proxmox_backup_manager/s3.rs
+++ b/src/bin/proxmox_backup_manager/s3.rs
@@ -19,6 +19,7 @@ use serde_json::Value;
             "store-prefix": {
                 type: String,
                 description: "Store prefix within bucket for S3 object keys (commonly datastore name)",
+                optional: true,
             },
         },
     },
@@ -27,7 +28,7 @@ use serde_json::Value;
 async fn check(
     s3_endpoint_id: String,
     bucket: String,
-    store_prefix: String,
+    store_prefix: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
     api2::admin::s3::check(s3_endpoint_id, bucket, store_prefix, rpcenv).await?;
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox-backup 4/6] api: config s3: add bucket list api endpoint

[Christian Ebner]

This endpoint allows to list buckets accessible given the provided
endpoint configuration. The bucket list is intended to be used for
ease of datastore setup by providing a list of bucket names to the
user in the frontend.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/config/s3.rs | 48 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
index 04b801028..047bf1fb1 100644
--- a/src/api2/config/s3.rs
+++ b/src/api2/config/s3.rs
@@ -5,8 +5,8 @@ use serde_json::Value;
 
 use proxmox_router::{http_bail, Permission, Router, RpcEnvironment};
 use proxmox_s3_client::{
-    S3ClientConf, S3ClientConfig, S3ClientConfigUpdater, S3ClientConfigWithoutSecret,
-    S3_CLIENT_ID_SCHEMA,
+    S3BucketListItem, S3Client, S3ClientConf, S3ClientConfig, S3ClientConfigUpdater,
+    S3ClientConfigWithoutSecret, S3ClientOptions, S3_CLIENT_ID_SCHEMA,
 };
 use proxmox_schema::{api, param_bail, ApiType};
 
@@ -273,6 +273,45 @@ pub fn delete_s3_client_config(
     s3::save_config(&config)
 }
 
+#[api(
+    input: {
+        properties: {
+            id: {
+                schema: S3_CLIENT_ID_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&[], PRIV_SYS_AUDIT, false),
+    },
+)]
+/// List buckets accessible by given s3 client configuration
+pub async fn list_buckets(
+    id: String,
+    _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<S3BucketListItem>, Error> {
+    let (config, _digest) = pbs_config::s3::config()?;
+    let config: S3ClientConf = config
+        .lookup(S3_CFG_TYPE_ID, &id)
+        .context("config lookup failed")?;
+
+    let empty_prefix = String::new();
+    let options =
+        S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix);
+    let client = S3Client::new(options).context("client creation failed")?;
+    let list_buckets_response = client
+        .list_buckets()
+        .await
+        .context("failed to list buckets")?;
+    let buckets = list_buckets_response
+        .buckets
+        .into_iter()
+        .map(|bucket| S3BucketListItem { name: bucket.name })
+        .collect();
+
+    Ok(buckets)
+}
+
 // Check if the configured s3 client is still in-use by a datastore backend.
 //
 // If so, return the first datastore name with the configured client.
@@ -294,10 +333,13 @@ fn s3_client_in_use(id: &str) -> Result<Option<String>, Error> {
     Ok(None)
 }
 
+const LIST_BUCKETS_ROUTER: Router = Router::new().get(&API_METHOD_LIST_BUCKETS);
+
 const ITEM_ROUTER: Router = Router::new()
     .get(&API_METHOD_READ_S3_CLIENT_CONFIG)
     .put(&API_METHOD_UPDATE_S3_CLIENT_CONFIG)
-    .delete(&API_METHOD_DELETE_S3_CLIENT_CONFIG);
+    .delete(&API_METHOD_DELETE_S3_CLIENT_CONFIG)
+    .subdirs(&[("list-buckets", &LIST_BUCKETS_ROUTER)]);
 
 pub const ROUTER: Router = Router::new()
     .get(&API_METHOD_LIST_S3_CLIENT_CONFIG)
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox-backup 5/6] bin: expose list buckets as proxmox-backup-manager s3 subcommand

[Christian Ebner]

Allows to list the buckets from the cli, mostly to be feature
compatible with the rest of the api endpoints for s3.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/bin/proxmox_backup_manager/s3.rs | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/bin/proxmox_backup_manager/s3.rs b/src/bin/proxmox_backup_manager/s3.rs
index f660baf4a..a94371e09 100644
--- a/src/bin/proxmox_backup_manager/s3.rs
+++ b/src/bin/proxmox_backup_manager/s3.rs
@@ -1,5 +1,5 @@
 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
-use proxmox_s3_client::{S3_BUCKET_NAME_SCHEMA, S3_CLIENT_ID_SCHEMA};
+use proxmox_s3_client::{S3BucketListItem, S3_BUCKET_NAME_SCHEMA, S3_CLIENT_ID_SCHEMA};
 use proxmox_schema::api;
 
 use proxmox_backup::api2;
@@ -35,6 +35,23 @@ async fn check(
     Ok(Value::Null)
 }
 
+#[api(
+    input: {
+        properties: {
+            "s3-endpoint-id": {
+                schema: S3_CLIENT_ID_SCHEMA,
+            },
+        },
+    },
+)]
+/// List buckets accessible by the given S3 client configuration
+async fn list_buckets(
+    s3_endpoint_id: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<S3BucketListItem>, Error> {
+    api2::config::s3::list_buckets(s3_endpoint_id, rpcenv).await
+}
+
 #[api(
     input: {
         properties: {
@@ -88,6 +105,12 @@ pub fn s3_commands() -> CommandLineInterface {
             CliCommand::new(&api2::config::s3::API_METHOD_DELETE_S3_CLIENT_CONFIG)
                 .arg_param(&["id"])
                 .completion_cb("id", pbs_config::s3::complete_s3_client_id),
+        )
+        .insert(
+            "list-buckets",
+            CliCommand::new(&API_METHOD_LIST_BUCKETS)
+                .arg_param(&["s3-endpoint-id"])
+                .completion_cb("s3-endpoint-id", pbs_config::s3::complete_s3_client_id),
         );
 
     let cmd_def = CliCommandMap::new()
-- 
2.47.2



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

[pbs-devel] [PATCH proxmox-backup 6/6] ui: replace bucket field by bucket selector

[Christian Ebner]

With the goal to improve usability for the datastore creation window,
replace the current bucket textfield with a bucket selector which
fetches available buckets on demand.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/Makefile                 |  1 +
 www/form/S3BucketSelector.js | 52 ++++++++++++++++++++++++++++++++++++
 www/window/DataStoreEdit.js  | 22 +++++++++++----
 3 files changed, 70 insertions(+), 5 deletions(-)
 create mode 100644 www/form/S3BucketSelector.js

diff --git a/www/Makefile b/www/Makefile
index 4a5cd1ff1..9ebf0445f 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -42,6 +42,7 @@ JSSRC=							\
 	Schema.js					\
 	form/TokenSelector.js				\
 	form/AuthidSelector.js				\
+	form/S3BucketSelector.js			\
 	form/S3ClientSelector.js			\
 	form/RemoteSelector.js				\
 	form/RemoteTargetSelector.js   			\
diff --git a/www/form/S3BucketSelector.js b/www/form/S3BucketSelector.js
new file mode 100644
index 000000000..75b02479f
--- /dev/null
+++ b/www/form/S3BucketSelector.js
@@ -0,0 +1,52 @@
+Ext.define('PBS.form.S3BucketSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.pbsS3BucketSelector',
+
+    allowBlank: false,
+    valueField: 'name',
+    displayField: 'name',
+
+    listConfig: {
+        width: 350,
+        columns: [
+            {
+                header: gettext('Bucket Name'),
+                sortable: true,
+                dataIndex: 'name',
+                renderer: Ext.String.htmlEncode,
+                flex: 1,
+            },
+        ],
+    },
+
+    store: {
+        autoLoad: false,
+        sorters: 'name',
+        fields: ['name'],
+        proxy: {
+            type: 'proxmox',
+            url: `/api2/json/config/s3/${encodeURIComponent(this.endpoint)}/list-buckets`,
+        },
+    },
+
+    setS3Endpoint: function (endpoint) {
+        let me = this;
+
+        if (me.endpoint === endpoint) {
+            return;
+        }
+
+        me.endpoint = endpoint;
+        me.store.removeAll();
+
+        me.setDisabled(false);
+
+        if (!me.firstLoad) {
+            me.clearValue();
+        }
+
+        me.store.proxy.url = `/api2/json/config/s3/${encodeURIComponent(me.endpoint)}/list-buckets`;
+        me.store.load();
+        me.firstLoad = false;
+    },
+});
diff --git a/www/window/DataStoreEdit.js b/www/window/DataStoreEdit.js
index 8296e0b04..1b935ddd3 100644
--- a/www/window/DataStoreEdit.js
+++ b/www/window/DataStoreEdit.js
@@ -74,7 +74,7 @@ Ext.define('PBS.DataStoreEdit', {
                                 let inputPanel = checkbox.up('inputpanel');
                                 let pathField = inputPanel.down('[name=path]');
                                 let uuidEditField = inputPanel.down('[name=backing-device]');
-                                let bucketField = inputPanel.down('[name=bucket]');
+                                let s3BucketSelector = inputPanel.down('[name=bucket]');
                                 let s3ClientSelector = inputPanel.down('[name=s3client]');
                                 let overwriteInUseField =
                                     inputPanel.down('[name=overwrite-in-use]');
@@ -86,9 +86,11 @@ Ext.define('PBS.DataStoreEdit', {
                                 uuidEditField.allowBlank = !isRemovable;
                                 uuidEditField.setValue('');
 
-                                bucketField.setDisabled(!isS3);
-                                bucketField.allowBlank = !isS3;
-                                bucketField.setValue('');
+                                if (!isS3) {
+                                    s3BucketSelector.setDisabled(true);
+                                    s3BucketSelector.setValue('');
+                                }
+                                s3BucketSelector.allowBlank = !isS3;
 
                                 s3ClientSelector.setDisabled(!isS3);
                                 s3ClientSelector.allowBlank = !isS3;
@@ -130,6 +132,13 @@ Ext.define('PBS.DataStoreEdit', {
                         cbind: {
                             editable: '{isCreate}',
                         },
+                        listeners: {
+                            change: function (selector, endpointId) {
+                                let inputPanel = selector.up('inputpanel');
+                                let s3BucketSelector = inputPanel.down('[name=bucket]');
+                                s3BucketSelector.setS3Endpoint(endpointId);
+                            },
+                        },
                     },
                 ],
                 column2: [
@@ -166,11 +175,14 @@ Ext.define('PBS.DataStoreEdit', {
                         emptyText: gettext('Device path'),
                     },
                     {
-                        xtype: 'proxmoxtextfield',
+                        xtype: 'pbsS3BucketSelector',
                         name: 'bucket',
                         fieldLabel: gettext('Bucket'),
                         allowBlank: false,
                         disabled: true,
+                        cbind: {
+                            editable: '{isCreate}',
+                        },
                     },
                 ],
                 columnB: [
-- 
2.47.2



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

Re: [pbs-devel] [PATCH proxmox{, -backup} 00/10] s3: implement list buckets and use bucket selector for datastore creation

[Lukas Wagner]

On Mon Jul 28, 2025 at 8:46 AM CEST, Christian Ebner wrote:
> This patches aim to increase usability for the user by allowing to list
> accessible buckets during datastore creation.
>
> For this, the proxmox s3 client is extended by the list buckets method, fetching
> accessible buckets from the s3 api. This requires the client instantiation to be
> possible without the bucket name, making it therefore optional. Further, the
> regex for the config endpoint url must be restricted to have the bucket name
> template pattern always at the start of the template. With these changes, it is
> then possible to fetch the buckets given the required permissions to do so.
>
> On the proxmox backup server side, the api endpoints to fetch the bucket list
> are implemented as well as the corresponding cli command. Finally, the bucket
> name field in the datastore edit window is replaced by an s3 bucket selector,
> which loads the accessible bucket names via the api.
>

Already reported to Chris off-list, but there seems to be something
wrong in the response parser when multiple buckets are returned:

$ proxmox-backup-manager s3 endpoint list-buckets minio
Error: failed to list buckets

Caused by:
    0: failed to parse response body
    1: custom: duplicate field `Bucket`

If I remove all but one buckets from the MinIO server, it works.


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

Re: [pbs-devel] [PATCH proxmox{, -backup} 00/10] s3: implement list buckets and use bucket selector for datastore creation

[Lukas Wagner]

On Mon Jul 28, 2025 at 8:46 AM CEST, Christian Ebner wrote:
> This patches aim to increase usability for the user by allowing to list
> accessible buckets during datastore creation.
>
> For this, the proxmox s3 client is extended by the list buckets method, fetching
> accessible buckets from the s3 api. This requires the client instantiation to be
> possible without the bucket name, making it therefore optional. Further, the
> regex for the config endpoint url must be restricted to have the bucket name
> template pattern always at the start of the template. With these changes, it is
> then possible to fetch the buckets given the required permissions to do so.
>
> On the proxmox backup server side, the api endpoints to fetch the bucket list
> are implemented as well as the corresponding cli command. Finally, the bucket
> name field in the datastore edit window is replaced by an s3 bucket selector,
> which loads the accessible bucket names via the api.
>

Apart from the issue I mentioned in my other mail, this looks good to
me:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>


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

Re: [pbs-devel] [PATCH proxmox{, -backup} 00/10] s3: implement list buckets and use bucket selector for datastore creation

[Christian Ebner]

On 7/30/25 3:46 PM, Lukas Wagner wrote:
> On Mon Jul 28, 2025 at 8:46 AM CEST, Christian Ebner wrote:
>> This patches aim to increase usability for the user by allowing to list
>> accessible buckets during datastore creation.
>>
>> For this, the proxmox s3 client is extended by the list buckets method, fetching
>> accessible buckets from the s3 api. This requires the client instantiation to be
>> possible without the bucket name, making it therefore optional. Further, the
>> regex for the config endpoint url must be restricted to have the bucket name
>> template pattern always at the start of the template. With these changes, it is
>> then possible to fetch the buckets given the required permissions to do so.
>>
>> On the proxmox backup server side, the api endpoints to fetch the bucket list
>> are implemented as well as the corresponding cli command. Finally, the bucket
>> name field in the datastore edit window is replaced by an s3 bucket selector,
>> which loads the accessible bucket names via the api.
>>
> 
> Already reported to Chris off-list, but there seems to be something
> wrong in the response parser when multiple buckets are returned:
> 
> $ proxmox-backup-manager s3 endpoint list-buckets minio
> Error: failed to list buckets
> 
> Caused by:
>      0: failed to parse response body
>      1: custom: duplicate field `Bucket`
> 
> If I remove all but one buckets from the MinIO server, it works.

Thanks for catching this!

Will be fixed in a version 2 of the patches. I will also work on some 
regression tests for the whole of the s3 client response parsing. This I 
will however send as followup patches.


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

[pbs-devel] superseded: [PATCH proxmox{, -backup} 00/10] s3: implement list buckets and use bucket selector for datastore creation

[Christian Ebner]

superseded-by version 2:
https://lore.proxmox.com/pbs-devel/20250730144008.642429-1-c.ebner@proxmox.com/T


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