public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH v3 proxmox-backup] fix #6185: client/docs: explicitly mention archive name restrictions
@ 2025-02-21 14:17 Christian Ebner
  2025-03-17 15:18 ` Fabian Grünbichler
  0 siblings, 1 reply; 3+ messages in thread
From: Christian Ebner @ 2025-02-21 14:17 UTC (permalink / raw)
  To: pbs-devel

Mention in the docs and the api parameter description the limitations
for archive name labels. They must contain alphanumerics, hyphens and
underscores only to match the regex pattern.

Further, explicitly mention the extension in the backup source schema
specification, to clearly distinguish this from the label and be in
line with the description give in the schema for the cli command.

By setting this in the api parameter description, it will be included
in the man page for proxmox-backup-client.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6185
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 2:
- Also explicitly mention the extension part in the backup source
  schema description, thanks @Shannon for noticing!
- Adapt commit message accordingly

 docs/backup-client.rst                 |  1 +
 pbs-client/src/backup_specification.rs | 10 ++++++----
 proxmox-backup-client/src/main.rs      |  3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index 1c2a98ed8..e11c0142a 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -169,6 +169,7 @@ the client. The format is:
 
     <archive-name>.<type>:<source-path>
 
+The ``archive-name`` must contain alphanumerics, hyphens and underscores only.
 Common types are ``.pxar`` for file archives and ``.img`` for block
 device images. To create a backup of a block device, run the following command:
 
diff --git a/pbs-client/src/backup_specification.rs b/pbs-client/src/backup_specification.rs
index b6fbc555c..fe5c352e4 100644
--- a/pbs-client/src/backup_specification.rs
+++ b/pbs-client/src/backup_specification.rs
@@ -7,10 +7,12 @@ const_regex! {
     BACKUPSPEC_REGEX = r"^([a-zA-Z0-9_-]+\.(pxar|img|conf|log)):(.+)$";
 }
 
-pub const BACKUP_SOURCE_SCHEMA: Schema =
-    StringSchema::new("Backup source specification ([<label>:<path>]).")
-        .format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
-        .schema();
+pub const BACKUP_SOURCE_SCHEMA: Schema = StringSchema::new(
+    "Backup source specification ([<label.ext>:<path>]), the specification \
+    'label' must contain alphanumerics, hyphens and underscores only.",
+)
+.format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
+.schema();
 
 pub enum BackupSpecificationType {
     PXAR,
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 589a097bb..9077f7bdf 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -628,7 +628,8 @@ fn spawn_catalog_upload(
        properties: {
            backupspec: {
                type: Array,
-               description: "List of backup source specifications ([<label.ext>:<path>] ...)",
+               description: "List of backup source specifications ([<label.ext>:<path>] ...), the \
+                    specifications 'label' must contain alphanumerics, hyphens and underscores only.",
                items: {
                    schema: BACKUP_SOURCE_SCHEMA,
                }
-- 
2.39.5



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


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

* Re: [pbs-devel] [PATCH v3 proxmox-backup] fix #6185: client/docs: explicitly mention archive name restrictions
  2025-02-21 14:17 [pbs-devel] [PATCH v3 proxmox-backup] fix #6185: client/docs: explicitly mention archive name restrictions Christian Ebner
@ 2025-03-17 15:18 ` Fabian Grünbichler
  2025-03-17 15:32   ` Christian Ebner
  0 siblings, 1 reply; 3+ messages in thread
From: Fabian Grünbichler @ 2025-03-17 15:18 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

On February 21, 2025 3:17 pm, Christian Ebner wrote:
> Mention in the docs and the api parameter description the limitations
> for archive name labels. They must contain alphanumerics, hyphens and
> underscores only to match the regex pattern.
> 
> Further, explicitly mention the extension in the backup source schema
> specification, to clearly distinguish this from the label and be in
> line with the description give in the schema for the cli command.
> 
> By setting this in the api parameter description, it will be included
> in the man page for proxmox-backup-client.
> 
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6185
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 2:
> - Also explicitly mention the extension part in the backup source
>   schema description, thanks @Shannon for noticing!
> - Adapt commit message accordingly
> 
>  docs/backup-client.rst                 |  1 +
>  pbs-client/src/backup_specification.rs | 10 ++++++----
>  proxmox-backup-client/src/main.rs      |  3 ++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/backup-client.rst b/docs/backup-client.rst
> index 1c2a98ed8..e11c0142a 100644
> --- a/docs/backup-client.rst
> +++ b/docs/backup-client.rst
> @@ -169,6 +169,7 @@ the client. The format is:
>  
>      <archive-name>.<type>:<source-path>
>  
> +The ``archive-name`` must contain alphanumerics, hyphens and underscores only.
>  Common types are ``.pxar`` for file archives and ``.img`` for block
>  device images. To create a backup of a block device, run the following command:

here we call it archive-name, type and source-path, but
>  
> diff --git a/pbs-client/src/backup_specification.rs b/pbs-client/src/backup_specification.rs
> index b6fbc555c..fe5c352e4 100644
> --- a/pbs-client/src/backup_specification.rs
> +++ b/pbs-client/src/backup_specification.rs
> @@ -7,10 +7,12 @@ const_regex! {
>      BACKUPSPEC_REGEX = r"^([a-zA-Z0-9_-]+\.(pxar|img|conf|log)):(.+)$";
>  }
>  
> -pub const BACKUP_SOURCE_SCHEMA: Schema =
> -    StringSchema::new("Backup source specification ([<label>:<path>]).")
> -        .format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
> -        .schema();
> +pub const BACKUP_SOURCE_SCHEMA: Schema = StringSchema::new(
> +    "Backup source specification ([<label.ext>:<path>]), the specification \
> +    'label' must contain alphanumerics, hyphens and underscores only.",
> +)

here we call it label, ext and path ? maybe we could unify these?

> +.format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
> +.schema();
>  
>  pub enum BackupSpecificationType {
>      PXAR,
> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 589a097bb..9077f7bdf 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -628,7 +628,8 @@ fn spawn_catalog_upload(
>         properties: {
>             backupspec: {
>                 type: Array,
> -               description: "List of backup source specifications ([<label.ext>:<path>] ...)",
> +               description: "List of backup source specifications ([<label.ext>:<path>] ...), the \
> +                    specifications 'label' must contain alphanumerics, hyphens and underscores only.",

same here, and the second 'specifications' should be singular like
above (or be dropped in both instances, since it doesn't really provide
any extra information over just "the 'label'" or "the 'archive-name'").

I think I'd also prefer "can only contain" or "must only contain"
instead of putting the 'only' at the end.

>                 items: {
>                     schema: BACKUP_SOURCE_SCHEMA,
>                 }
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


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


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

* Re: [pbs-devel] [PATCH v3 proxmox-backup] fix #6185: client/docs: explicitly mention archive name restrictions
  2025-03-17 15:18 ` Fabian Grünbichler
@ 2025-03-17 15:32   ` Christian Ebner
  0 siblings, 0 replies; 3+ messages in thread
From: Christian Ebner @ 2025-03-17 15:32 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Fabian Grünbichler

On 3/17/25 16:18, Fabian Grünbichler wrote:
> On February 21, 2025 3:17 pm, Christian Ebner wrote:
>> Mention in the docs and the api parameter description the limitations
>> for archive name labels. They must contain alphanumerics, hyphens and
>> underscores only to match the regex pattern.
>>
>> Further, explicitly mention the extension in the backup source schema
>> specification, to clearly distinguish this from the label and be in
>> line with the description give in the schema for the cli command.
>>
>> By setting this in the api parameter description, it will be included
>> in the man page for proxmox-backup-client.
>>
>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6185
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 2:
>> - Also explicitly mention the extension part in the backup source
>>    schema description, thanks @Shannon for noticing!
>> - Adapt commit message accordingly
>>
>>   docs/backup-client.rst                 |  1 +
>>   pbs-client/src/backup_specification.rs | 10 ++++++----
>>   proxmox-backup-client/src/main.rs      |  3 ++-
>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/backup-client.rst b/docs/backup-client.rst
>> index 1c2a98ed8..e11c0142a 100644
>> --- a/docs/backup-client.rst
>> +++ b/docs/backup-client.rst
>> @@ -169,6 +169,7 @@ the client. The format is:
>>   
>>       <archive-name>.<type>:<source-path>
>>   
>> +The ``archive-name`` must contain alphanumerics, hyphens and underscores only.
>>   Common types are ``.pxar`` for file archives and ``.img`` for block
>>   device images. To create a backup of a block device, run the following command:
> 
> here we call it archive-name, type and source-path, but
>>   
>> diff --git a/pbs-client/src/backup_specification.rs b/pbs-client/src/backup_specification.rs
>> index b6fbc555c..fe5c352e4 100644
>> --- a/pbs-client/src/backup_specification.rs
>> +++ b/pbs-client/src/backup_specification.rs
>> @@ -7,10 +7,12 @@ const_regex! {
>>       BACKUPSPEC_REGEX = r"^([a-zA-Z0-9_-]+\.(pxar|img|conf|log)):(.+)$";
>>   }
>>   
>> -pub const BACKUP_SOURCE_SCHEMA: Schema =
>> -    StringSchema::new("Backup source specification ([<label>:<path>]).")
>> -        .format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
>> -        .schema();
>> +pub const BACKUP_SOURCE_SCHEMA: Schema = StringSchema::new(
>> +    "Backup source specification ([<label.ext>:<path>]), the specification \
>> +    'label' must contain alphanumerics, hyphens and underscores only.",
>> +)
> 
> here we call it label, ext and path ? maybe we could unify these?

Okay, this patch has been applied already [0] but I can send a followup 
for adaption according to your suggestions.

> 
>> +.format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
>> +.schema();
>>   
>>   pub enum BackupSpecificationType {
>>       PXAR,
>> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
>> index 589a097bb..9077f7bdf 100644
>> --- a/proxmox-backup-client/src/main.rs
>> +++ b/proxmox-backup-client/src/main.rs
>> @@ -628,7 +628,8 @@ fn spawn_catalog_upload(
>>          properties: {
>>              backupspec: {
>>                  type: Array,
>> -               description: "List of backup source specifications ([<label.ext>:<path>] ...)",
>> +               description: "List of backup source specifications ([<label.ext>:<path>] ...), the \
>> +                    specifications 'label' must contain alphanumerics, hyphens and underscores only.",
> 
> same here, and the second 'specifications' should be singular like
> above (or be dropped in both instances, since it doesn't really provide
> any extra information over just "the 'label'" or "the 'archive-name'").
> 
> I think I'd also prefer "can only contain" or "must only contain"
> instead of putting the 'only' at the end.

Acked.

> 
>>                  items: {
>>                      schema: BACKUP_SOURCE_SCHEMA,
>>                  }
>> -- 
>> 2.39.5
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 

[0] 
https://git.proxmox.com/?p=proxmox-backup.git;a=commit;h=abad8e25c46f66106ec07bb3381e63fe8c12f1dd


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

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

end of thread, other threads:[~2025-03-17 15:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-21 14:17 [pbs-devel] [PATCH v3 proxmox-backup] fix #6185: client/docs: explicitly mention archive name restrictions Christian Ebner
2025-03-17 15:18 ` Fabian Grünbichler
2025-03-17 15:32   ` 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