[pbs-devel] [PATCH backup v2 1/7] pbs-client: use a const for the PBS_REPOSITORY env variable

[pbs-devel] [PATCH backup v2 1/7] pbs-client: use a const for the PBS_REPOSITORY env variable

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Differences from v1:
- Use the helper on get_encryption_password
- Send "add helper" commit to the past

 pbs-client/src/tools/mod.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 3b19df39..a42fa114 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -29,6 +29,7 @@ pub mod key_source;
 const ENV_VAR_PBS_FINGERPRINT: &str = "PBS_FINGERPRINT";
 const ENV_VAR_PBS_PASSWORD: &str = "PBS_PASSWORD";
 const ENV_VAR_PBS_ENCRYPTION_PASSWORD: &str = "PBS_ENCRYPTION_PASSWORD";
+const ENV_VAR_PBS_REPOSITORY: &str = "PBS_REPOSITORY";
 
 /// Directory with system [credential]s. See systemd-creds(1).
 ///
@@ -195,7 +196,7 @@ pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
 }
 
 pub fn get_default_repository() -> Option<String> {
-    std::env::var("PBS_REPOSITORY").ok()
+    std::env::var(ENV_VAR_PBS_REPOSITORY).ok()
 }
 
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
-- 
2.39.5



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

[pbs-devel] [PATCH backup v2 2/7] pbs-client: add helper for getting UTF-8 secrets

[Maximiliano Sandoval]

We are going to add more credentials so it makes sense to have a common
helper to get the secrets.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index a42fa114..efd2e139 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -153,6 +153,25 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
     Ok(None)
 }
 
+/// Gets a secret or value from the environment.
+///
+/// Checks for an environment variable named `env_variable`, and if missing, it
+/// checks for a system [credential] named `credential_name`. Assumes the secret
+/// is UTF-8 encoded.
+///
+/// [credential]: https://systemd.io/CREDENTIALS/
+fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
+    if let Some(password) = get_secret_from_env(env_variable)? {
+        Ok(Some(password))
+    } else if let Some(password) = get_credential(credential_name)? {
+        String::from_utf8(password)
+            .map(Option::Some)
+            .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
+    } else {
+        Ok(None)
+    }
+}
+
 /// Gets the backup server's password.
 ///
 /// Looks for a password in the `PBS_PASSWORD` environment variable, if there
-- 
2.39.5



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

Re: [pbs-devel] [PATCH backup v2 2/7] pbs-client: add helper for getting UTF-8 secrets

[Christian Ebner]

On 3/27/25 11:47, Maximiliano Sandoval wrote:
> We are going to add more credentials so it makes sense to have a common
> helper to get the secrets.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>   pbs-client/src/tools/mod.rs | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index a42fa114..efd2e139 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -153,6 +153,25 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
>       Ok(None)
>   }
>   
> +/// Gets a secret or value from the environment.
> +///
> +/// Checks for an environment variable named `env_variable`, and if missing, it
> +/// checks for a system [credential] named `credential_name`. Assumes the secret
> +/// is UTF-8 encoded.
> +///
> +/// [credential]: https://systemd.io/CREDENTIALS/
> +fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
> +    if let Some(password) = get_secret_from_env(env_variable)? {
> +        Ok(Some(password))
> +    } else if let Some(password) = get_credential(credential_name)? {
> +        String::from_utf8(password)
> +            .map(Option::Some)
> +            .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))

This check for valid UTF-8 could rather happen directly in 
get_credential since you will enforce get_encryption_password to return 
a String anyways in patch 4? And the others already return a String anyways.

Only allowing UTF-8 content from get_credential would bring us in line 
with secrets obtained from the other sources, and since this has been 
introduced only recently (and never packaged until now), this could 
still be changed without break changes.

As a side effect this helper would than simplify further.

> +    } else {
> +        Ok(None)
> +    }
> +}
> +
>   /// Gets the backup server's password.
>   ///
>   /// Looks for a password in the `PBS_PASSWORD` environment variable, if there



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

Re: [pbs-devel] [PATCH backup v2 2/7] pbs-client: add helper for getting UTF-8 secrets

[Maximiliano Sandoval]

Christian Ebner <c.ebner@proxmox.com> writes:

> On 3/27/25 11:47, Maximiliano Sandoval wrote:
>> We are going to add more credentials so it makes sense to have a common
>> helper to get the secrets.
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>>   pbs-client/src/tools/mod.rs | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
>> index a42fa114..efd2e139 100644
>> --- a/pbs-client/src/tools/mod.rs
>> +++ b/pbs-client/src/tools/mod.rs
>> @@ -153,6 +153,25 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
>>       Ok(None)
>>   }
>>   +/// Gets a secret or value from the environment.
>> +///
>> +/// Checks for an environment variable named `env_variable`, and if missing, it
>> +/// checks for a system [credential] named `credential_name`. Assumes the secret
>> +/// is UTF-8 encoded.
>> +///
>> +/// [credential]: https://systemd.io/CREDENTIALS/
>> +fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
>> +    if let Some(password) = get_secret_from_env(env_variable)? {
>> +        Ok(Some(password))
>> +    } else if let Some(password) = get_credential(credential_name)? {
>> +        String::from_utf8(password)
>> +            .map(Option::Some)
>> +            .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
>
> This check for valid UTF-8 could rather happen directly in get_credential since
> you will enforce get_encryption_password to return a String anyways in patch 4?
> And the others already return a String anyways.

Yes, you could. However, our secrets being valid UTF-8 is an
implementation detail that I would prefer to not leak onto that layer of
the API, credentials are intrinsically arbitrary bytes (and sometimes
UTF-8 text) and imho the get_credential function should reflect that.

> Only allowing UTF-8 content from get_credential would bring us in line with
> secrets obtained from the other sources, and since this has been introduced only
> recently (and never packaged until now), this could still be changed without
> break changes.
>
> As a side effect this helper would than simplify further.
>
>> +    } else {
>> +        Ok(None)
>> +    }
>> +}
>> +
>>   /// Gets the backup server's password.
>>   ///
>>   /// Looks for a password in the `PBS_PASSWORD` environment variable, if there



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

Re: [pbs-devel] [PATCH backup v2 2/7] pbs-client: add helper for getting UTF-8 secrets

[Christian Ebner]

On 3/27/25 13:16, Maximiliano Sandoval wrote:
> 
> Christian Ebner <c.ebner@proxmox.com> writes:
> 
>> On 3/27/25 11:47, Maximiliano Sandoval wrote:
>>> We are going to add more credentials so it makes sense to have a common
>>> helper to get the secrets.
>>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>>> ---
>>>    pbs-client/src/tools/mod.rs | 19 +++++++++++++++++++
>>>    1 file changed, 19 insertions(+)
>>> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
>>> index a42fa114..efd2e139 100644
>>> --- a/pbs-client/src/tools/mod.rs
>>> +++ b/pbs-client/src/tools/mod.rs
>>> @@ -153,6 +153,25 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
>>>        Ok(None)
>>>    }
>>>    +/// Gets a secret or value from the environment.
>>> +///
>>> +/// Checks for an environment variable named `env_variable`, and if missing, it
>>> +/// checks for a system [credential] named `credential_name`. Assumes the secret
>>> +/// is UTF-8 encoded.
>>> +///
>>> +/// [credential]: https://systemd.io/CREDENTIALS/
>>> +fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
>>> +    if let Some(password) = get_secret_from_env(env_variable)? {
>>> +        Ok(Some(password))
>>> +    } else if let Some(password) = get_credential(credential_name)? {
>>> +        String::from_utf8(password)
>>> +            .map(Option::Some)
>>> +            .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
>>
>> This check for valid UTF-8 could rather happen directly in get_credential since
>> you will enforce get_encryption_password to return a String anyways in patch 4?
>> And the others already return a String anyways.
> 
> Yes, you could. However, our secrets being valid UTF-8 is an
> implementation detail that I would prefer to not leak onto that layer of
> the API, credentials are intrinsically arbitrary bytes (and sometimes
> UTF-8 text) and imho the get_credential function should reflect that.

Fine by me, although I do not really see that as an issue here. 
get_credential is a private helper without external call sides. Also, 
refactoring this will still be possible in the future if other use cases 
arise requiring the raw bytes instead of the encoded string.


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

[pbs-devel] [PATCH backup v2 3/7] pbs-client: use helper for getting UTF-8 password

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index efd2e139..81e29ffe 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -182,15 +182,7 @@ fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<S
 ///
 /// [credential]: https://systemd.io/CREDENTIALS/
 pub fn get_password() -> Result<Option<String>, Error> {
-    if let Some(password) = get_secret_from_env(ENV_VAR_PBS_PASSWORD)? {
-        Ok(Some(password))
-    } else if let Some(password) = get_credential(CRED_PBS_PASSWORD)? {
-        String::from_utf8(password)
-            .map(Option::Some)
-            .map_err(|_err| format_err!("non-utf8 password credential"))
-    } else {
-        Ok(None)
-    }
+    get_secret_impl(ENV_VAR_PBS_PASSWORD, CRED_PBS_PASSWORD)
 }
 
 /// Gets an encryption password.
-- 
2.39.5



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

[pbs-devel] [PATCH backup v2 4/7] pbs-client: make get_encryption_password return a String

[Maximiliano Sandoval]

As per the note in the documentation [1], passwords are valid UTF-8.
This allows us to se the shared helper.

[1] https://pbs.proxmox.com/docs/backup-client.html#environment-variables

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/key_source.rs |  2 +-
 pbs-client/src/tools/mod.rs        | 13 +++++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/pbs-client/src/tools/key_source.rs b/pbs-client/src/tools/key_source.rs
index 94b86e8b..9d5110e2 100644
--- a/pbs-client/src/tools/key_source.rs
+++ b/pbs-client/src/tools/key_source.rs
@@ -346,7 +346,7 @@ pub fn get_encryption_key_password() -> Result<Vec<u8>, Error> {
     // fixme: implement other input methods
 
     if let Some(password) = super::get_encryption_password()? {
-        return Ok(password);
+        return Ok(password.into_bytes());
     }
 
     // If we're on a TTY, query the user for a password
diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 81e29ffe..c5c0b34c 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -196,14 +196,11 @@ pub fn get_password() -> Result<Option<String>, Error> {
 /// present.
 ///
 /// [credential]: https://systemd.io/CREDENTIALS/
-pub fn get_encryption_password() -> Result<Option<Vec<u8>>, Error> {
-    if let Some(password) = get_secret_from_env(ENV_VAR_PBS_ENCRYPTION_PASSWORD)? {
-        Ok(Some(password.into_bytes()))
-    } else if let Some(password) = get_credential(CRED_PBS_ENCRYPTION_PASSWORD)? {
-        Ok(Some(password))
-    } else {
-        Ok(None)
-    }
+pub fn get_encryption_password() -> Result<Option<String>, Error> {
+    get_secret_impl(
+        ENV_VAR_PBS_ENCRYPTION_PASSWORD,
+        CRED_PBS_ENCRYPTION_PASSWORD,
+    )
 }
 
 pub fn get_default_repository() -> Option<String> {
-- 
2.39.5



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

[pbs-devel] [PATCH backup v2 5/7] pbs-client: allow reading default repository from system credential

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index c5c0b34c..fd08dc68 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -39,6 +39,8 @@ const ENV_VAR_CREDENTIALS_DIRECTORY: &str = "CREDENTIALS_DIRECTORY";
 const CRED_PBS_ENCRYPTION_PASSWORD: &str = "proxmox-backup-client.encryption-password";
 /// Credential name of the the password.
 const CRED_PBS_PASSWORD: &str = "proxmox-backup-client.password";
+/// Credential name of the the repository.
+const CRED_PBS_REPOSITORY: &str = "proxmox-backup-client.repository";
 
 pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
     .format(&BACKUP_REPO_URL)
@@ -204,7 +206,11 @@ pub fn get_encryption_password() -> Result<Option<String>, Error> {
 }
 
 pub fn get_default_repository() -> Option<String> {
-    std::env::var(ENV_VAR_PBS_REPOSITORY).ok()
+    get_secret_impl(ENV_VAR_PBS_REPOSITORY, CRED_PBS_REPOSITORY)
+        .inspect_err(|err| {
+            proxmox_log::error!("could not read default repository: {err:#}");
+        })
+        .unwrap_or_default()
 }
 
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
-- 
2.39.5



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

[pbs-devel] [PATCH backup v2 6/7] pbs-client: allow reading fingerprint from system credential

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index fd08dc68..f4b655e8 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -41,6 +41,8 @@ const CRED_PBS_ENCRYPTION_PASSWORD: &str = "proxmox-backup-client.encryption-pas
 const CRED_PBS_PASSWORD: &str = "proxmox-backup-client.password";
 /// Credential name of the the repository.
 const CRED_PBS_REPOSITORY: &str = "proxmox-backup-client.repository";
+/// Credential name of the the fingerprint.
+const CRED_PBS_FINGERPRINT: &str = "proxmox-backup-client.fingerprint";
 
 pub const REPO_URL_SCHEMA: Schema = StringSchema::new("Repository URL.")
     .format(&BACKUP_REPO_URL)
@@ -213,6 +215,24 @@ pub fn get_default_repository() -> Option<String> {
         .unwrap_or_default()
 }
 
+/// Gets the repository fingerprint.
+///
+/// Looks for the fingerprint in the `PBS_FINGERPRINT` environment variable, if
+/// there isn't one it reads the `proxmox-backup-client.fingerprint`
+/// [credential].
+///
+/// Returns `None` if neither the environment variable or the credential are
+/// present.
+///
+/// [credential]: https://systemd.io/CREDENTIALS/
+pub fn get_fingerprint() -> Option<String> {
+    get_secret_impl(ENV_VAR_PBS_FINGERPRINT, CRED_PBS_FINGERPRINT)
+        .inspect_err(|err| {
+            proxmox_log::error!("could not read fingerprint: {err:#}");
+        })
+        .unwrap_or_default()
+}
+
 pub fn remove_repository_from_value(param: &mut Value) -> Result<BackupRepository, Error> {
     if let Some(url) = param
         .as_object_mut()
@@ -270,7 +290,7 @@ fn connect_do(
     auth_id: &Authid,
     rate_limit: RateLimitConfig,
 ) -> Result<HttpClient, Error> {
-    let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
+    let fingerprint = get_fingerprint();
 
     let password = get_password()?;
     let options = HttpClientOptions::new_interactive(password, fingerprint).rate_limit(rate_limit);
@@ -280,7 +300,7 @@ fn connect_do(
 
 /// like get, but simply ignore errors and return Null instead
 pub async fn try_get(repo: &BackupRepository, url: &str) -> Value {
-    let fingerprint = std::env::var(ENV_VAR_PBS_FINGERPRINT).ok();
+    let fingerprint = get_fingerprint();
     let password = get_password().unwrap_or(None);
 
     // ticket cache, but no questions asked
-- 
2.39.5



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

[pbs-devel] [PATCH backup v2 7/7] docs: client: add section about system credentials

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 docs/backup-client.rst | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index e11c0142..aea63bd1 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -44,6 +44,9 @@ user\@pbs!token@host:store       ``user@pbs!token`` host:8007          store
 [ff80::51]:1234:mydatastore      ``root@pam``       [ff80::51]:1234    mydatastore
 ================================ ================== ================== ===========
 
+
+.. _environment-variables:
+
 Environment Variables
 ---------------------
 
@@ -89,6 +92,39 @@ Environment Variables
    you can add arbitrary comments after the first newline.
 
 
+System Credentials
+------------------
+
+Some of the :ref:`environment variables <environment-variables>` above can be
+set using `system credentials <https://systemd.io/CREDENTIALS/>`_ instead.
+
+============================ ==============================================
+Environment Variable         Credential Name Equivalent
+============================ ==============================================
+``PBS_REPOSITORY``           ``proxmox-backup-client.repository``
+``PBS_PASSWORD``             ``proxmox-backup-client.password``
+``PBS_ENCRYPTION_PASSWORD``  ``proxmox-backup-client.encryption-password``
+``PBS_FINGERPRINT``          ``proxmox-backup-client.fingerprint``
+============================ ==============================================
+
+For example, a credential for the repository password can be stored in an
+encrypted file as follows:
+
+.. code-block:: console
+
+  # systemd-ask-password -n | systemd-creds encrypt --name=proxmox-backup-client.password - my-api-token.cred
+
+The credential can be then reused inside of unit files or in a transient scope
+unit as follows:
+
+.. code-block:: console
+
+  # systemd-run --pipe --wait \
+  --property=LoadCredentialEncrypted=proxmox-backup-client.password:my-api-token.cred \
+  --property=SetCredential=proxmox-backup-client.repository:'my_default_repository' \
+  proxmox-backup-client ...
+
+
 Output Format
 -------------
 
-- 
2.39.5



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

Re: [pbs-devel] [PATCH backup v2 7/7] docs: client: add section about system credentials

[Christian Ebner]

some nits inline

On 3/27/25 11:47, Maximiliano Sandoval wrote:
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>   docs/backup-client.rst | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/docs/backup-client.rst b/docs/backup-client.rst
> index e11c0142..aea63bd1 100644
> --- a/docs/backup-client.rst
> +++ b/docs/backup-client.rst
> @@ -44,6 +44,9 @@ user\@pbs!token@host:store       ``user@pbs!token`` host:8007          store
>   [ff80::51]:1234:mydatastore      ``root@pam``       [ff80::51]:1234    mydatastore
>   ================================ ================== ================== ===========
>   
> +
> +.. _environment-variables:
> +
>   Environment Variables
>   ---------------------
>   
> @@ -89,6 +92,39 @@ Environment Variables
>      you can add arbitrary comments after the first newline.
>   
>   
> +System Credentials
> +------------------
> +
> +Some of the :ref:`environment variables <environment-variables>` above can be
> +set using `system credentials <https://systemd.io/CREDENTIALS/>`_ instead.
> +
> +============================ ==============================================
> +Environment Variable         Credential Name Equivalent
> +============================ ==============================================
> +``PBS_REPOSITORY``           ``proxmox-backup-client.repository``
> +``PBS_PASSWORD``             ``proxmox-backup-client.password``
> +``PBS_ENCRYPTION_PASSWORD``  ``proxmox-backup-client.encryption-password``
> +``PBS_FINGERPRINT``          ``proxmox-backup-client.fingerprint``
> +============================ ==============================================
> +
> +For example, a credential for the repository password can be stored in an

this sounds a bit redundant, maybe just
```
For example, the repository password can ...
```

> +encrypted file as follows:
> +
> +.. code-block:: console
> +
> +  # systemd-ask-password -n | systemd-creds encrypt --name=proxmox-backup-client.password - my-api-token.cred
> +
> +The credential can be then reused inside of unit files or in a transient scope

The credential can then be reused ...

> +unit as follows:
> +
> +.. code-block:: console
> +
> +  # systemd-run --pipe --wait \
> +  --property=LoadCredentialEncrypted=proxmox-backup-client.password:my-api-token.cred \

This required the full path to the encrypted file to work as expected, 
so maybe that should be mentioned as otherwise this trips up first users 
(me included).

> +  --property=SetCredential=proxmox-backup-client.repository:'my_default_repository' \
> +  proxmox-backup-client ...
> +
> +
>   Output Format
>   -------------
>

Further, it might be nice to have an example on how to invoke the client 
if the credentials are passed in as system credentials instead, e.g.
```
systemd-run --pipe --wait \\
     --property=LoadCredential=proxmox-backup-client.repository \\
     --property=LoadCredential=proxmox-backup-client.password \\
     --property=LoadCredential=proxmox-backup-client.encryption-password \\
     --property=LoadCredential=proxmox-backup-client.fingerprint \\
     proxmox-backup-client ...
```



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

Re: [pbs-devel] [PATCH backup v2 1/7] pbs-client: use a const for the PBS_REPOSITORY env variable

[Christian Ebner]

Tested by providing the repository and fingerprint by setting via 
`SetCredential` as well as by passing it to a VM via smbios values and 
loading them via `LoadCredential` on command invocation.
Some patches of the series could be squashed together IMO, but that is 
up for the maintainer to decide.

Otherwise LGTM, with the smaller issues addressed consider this:

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>


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