public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation
@ 2025-07-31 12:58 Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 1/4] s3 client: restrict bucket template pattern to start of endpoint url Christian Ebner
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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.

Changes since version 3 (thanks @Lukas for additional review and testing):
- Added additional doc comments
- Refactor response status code check and list buckets response generation

Changes since version 2:
- Rebased onto current master

Changes since version 1 (thanks @Lukas for review and testing:
- Fixed issues when listing multiple buckets

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          | 43 +++++++++++++----
 proxmox-s3-client/src/response_reader.rs | 61 ++++++++++++++++++++++++
 4 files changed, 112 insertions(+), 11 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       |  4 +--
 src/api2/admin/s3.rs                 |  6 ++--
 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 +++++++++++----
 8 files changed, 154 insertions(+), 17 deletions(-)
 create mode 100644 www/form/S3BucketSelector.js


Summary over all repositories:
  12 files changed, 266 insertions(+), 28 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox v4 1/4] s3 client: restrict bucket template pattern to start of endpoint url
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 2/4] s3 client: make bucket name optional in S3 client options Christian Ebner
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox v4 2/4] s3 client: make bucket name optional in S3 client options
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 1/4] s3 client: restrict bucket template pattern to start of endpoint url Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 3/4] s3 client: implement list buckets method Christian Ebner
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- cargo fmt

 proxmox-s3-client/examples/s3_client.rs |  2 +-
 proxmox-s3-client/src/client.rs         | 29 ++++++++++++++++++-------
 2 files changed, 22 insertions(+), 9 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..5c07671c 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,15 @@ impl S3Client {
         } else {
             options.endpoint.clone()
         };
-        let authority = authority_template
-            .replace("{{bucket}}", &options.bucket)
-            .replace("{{region}}", &options.region);
+
+        let authority = authority_template.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 +458,11 @@ impl S3Client {
         source_key: S3ObjectKey,
         destination_key: S3ObjectKey,
     ) -> Result<CopyObjectResponse, Error> {
-        let copy_source =
-            source_key.to_copy_source_key(&self.options.bucket, &self.options.common_prefix);
+        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(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 +661,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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox v4 3/4] s3 client: implement list buckets method
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 1/4] s3 client: restrict bucket template pattern to start of endpoint url Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 2/4] s3 client: make bucket name optional in S3 client options Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 4/4] s3 client: api types: add bucket list item type Christian Ebner
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- added missing doc comments
- refactored response parsing as suggested

 proxmox-s3-client/src/client.rs          | 14 +++++-
 proxmox-s3-client/src/response_reader.rs | 61 ++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
index 5c07671c..ae61c590 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);
@@ -318,6 +318,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..79a61cea 100644
--- a/proxmox-s3-client/src/response_reader.rs
+++ b/proxmox-s3-client/src/response_reader.rs
@@ -153,6 +153,44 @@ pub struct CopyObjectResult {
     pub last_modified: LastModifiedTimestamp,
 }
 
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+/// Parsed response of the list buckets api call
+pub struct ListBucketsResponse {
+    /// List of buckets
+    pub buckets: Vec<Bucket>,
+}
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+/// Subset of the list buckets response used to deserialize the buckets tag.
+/// https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html#API_ListBuckets_ResponseElements
+pub struct ListAllMyBucketsResult {
+    /// Buckets field.
+    pub buckets: Option<Buckets>,
+}
+
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+/// Subset used to deserialize the list of bucket tags in list buckets response.
+pub struct Buckets {
+    /// List of buckets.
+    pub bucket: Vec<Bucket>,
+}
+
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "PascalCase")]
+/// Subset used to deserialize bucket tag contents in the list buckets response.
+pub struct Bucket {
+    /// Bucket name.
+    pub name: String,
+    /// Bucket ARN.
+    pub bucket_arn: Option<String>,
+    /// Bucket region.
+    pub bucket_region: Option<String>,
+    /// Bucket creation date.
+    pub creation_date: LastModifiedTimestamp,
+}
+
 impl ResponseReader {
     pub(crate) fn new(response: Response<Incoming>) -> Self {
         Self { response }
@@ -339,6 +377,29 @@ impl ResponseReader {
         })
     }
 
+    /// Response parser for list buckets api calls.
+    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();
+
+        if !matches!(parts.status, StatusCode::OK) {
+            Self::log_error_response_utf8(body);
+            bail!("unexpected status code {}", parts.status);
+        };
+
+        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
+            .map(|b| b.bucket)
+            .unwrap_or_default();
+
+        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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox v4 4/4] s3 client: api types: add bucket list item type
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (2 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 3/4] s3 client: implement list buckets method Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 1/6] ui: fix formatting issues using proxmox-biome Christian Ebner
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v4 1/6] ui: fix formatting issues using proxmox-biome
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (3 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 4/4] s3 client: api types: add bucket list item type Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 2/6] datastore: wrap bucket name, as in is now optional in the s3 client Christian Ebner
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v4 2/6] datastore: wrap bucket name, as in is now optional in the s3 client
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (4 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 1/6] ui: fix formatting issues using proxmox-biome Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 3/6] api: admin s3: make store prefix for check command optional Christian Ebner
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 pbs-datastore/src/datastore.rs | 4 ++--
 src/api2/admin/s3.rs           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 57a90971a..5a22ffbcc 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -258,7 +258,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)?;
@@ -2433,7 +2433,7 @@ impl DataStore {
         let options = S3ClientOptions::from_config(
             client_config.config,
             client_config.secret_key,
-            bucket,
+            Some(bucket),
             datastore_config.name.to_owned(),
         );
         let s3_client = S3Client::new(options).context("failed to create s3 client")?;
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")?;
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v4 3/6] api: admin s3: make store prefix for check command optional
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (5 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 2/6] datastore: wrap bucket name, as in is now optional in the s3 client Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 4/6] api: config s3: add bucket list api endpoint Christian Ebner
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v4 4/6] api: config s3: add bucket list api endpoint
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (6 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 3/6] api: admin s3: make store prefix for check command optional Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 5/6] bin: expose list buckets as proxmox-backup-manager s3 subcommand Christian Ebner
  2025-07-31 12:59 ` [pbs-devel] [PATCH proxmox-backup v4 6/6] ui: replace bucket field by bucket selector Christian Ebner
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v4 5/6] bin: expose list buckets as proxmox-backup-manager s3 subcommand
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (7 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 4/6] api: config s3: add bucket list api endpoint Christian Ebner
@ 2025-07-31 12:58 ` Christian Ebner
  2025-07-31 12:59 ` [pbs-devel] [PATCH proxmox-backup v4 6/6] ui: replace bucket field by bucket selector Christian Ebner
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:58 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v4 6/6] ui: replace bucket field by bucket selector
  2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
                   ` (8 preceding siblings ...)
  2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 5/6] bin: expose list buckets as proxmox-backup-manager s3 subcommand Christian Ebner
@ 2025-07-31 12:59 ` Christian Ebner
  9 siblings, 0 replies; 11+ messages in thread
From: Christian Ebner @ 2025-07-31 12:59 UTC (permalink / raw)
  To: pbs-devel

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>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
---
changes since version 3:
- no changes

 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-07-31 12:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-31 12:58 [pbs-devel] [PATCH proxmox{, -backup} v4 00/10] s3: implement list buckets and use bucket selector for datastore creation Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 1/4] s3 client: restrict bucket template pattern to start of endpoint url Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 2/4] s3 client: make bucket name optional in S3 client options Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 3/4] s3 client: implement list buckets method Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox v4 4/4] s3 client: api types: add bucket list item type Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 1/6] ui: fix formatting issues using proxmox-biome Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 2/6] datastore: wrap bucket name, as in is now optional in the s3 client Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 3/6] api: admin s3: make store prefix for check command optional Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 4/6] api: config s3: add bucket list api endpoint Christian Ebner
2025-07-31 12:58 ` [pbs-devel] [PATCH proxmox-backup v4 5/6] bin: expose list buckets as proxmox-backup-manager s3 subcommand Christian Ebner
2025-07-31 12:59 ` [pbs-devel] [PATCH proxmox-backup v4 6/6] ui: replace bucket field by bucket selector Christian Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal