[pbs-devel] [PATCH proxmox-backup 0/4] close #3162: add options to configure mail_from address

[pbs-devel] [PATCH proxmox-backup 0/4] close #3162: add options to configure mail_from address

[Hannes Laimer]

Adds an option to configure the email address that is used as 'from'
when mails are sent.

Hannes Laimer (4):
  config: add email-from to NodeConfig
  api2: make email-from updatable
  server: use configured email-from for sending mail
  ui: add new options tab under configuration

 src/api2/node/config.rs           |  4 ++++
 src/config/node.rs                |  8 ++++++++
 src/server/email_notifications.rs |  6 +++++-
 www/SystemConfiguration.js        | 25 +++++++++++++++++++++++--
 www/config/NodeOptionView.js      |  9 +++++++++
 5 files changed, 49 insertions(+), 3 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 1/4] config: add email-from to NodeConfig

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/config/node.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/config/node.rs b/src/config/node.rs
index ebbd08bd..2056fea8 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -7,6 +7,7 @@ use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
 
 use proxmox_http::ProxyConfig;
 
+use pbs_api_types::EMAIL_SCHEMA;
 use pbs_buildcfg::configdir;
 use pbs_config::{open_backup_lockfile, BackupLockGuard};
 
@@ -86,6 +87,10 @@ pub struct AcmeConfig {
             schema: HTTP_PROXY_SCHEMA,
             optional: true,
         },
+        "email-from": {
+            schema: EMAIL_SCHEMA,
+            optional: true,
+        },
     },
 )]
 #[derive(Deserialize, Serialize, Updater)]
@@ -113,6 +118,9 @@ pub struct NodeConfig {
 
     #[serde(skip_serializing_if = "Option::is_none")]
     pub http_proxy: Option<String>,
+    
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub email_from: Option<String>,
 }
 
 impl NodeConfig {
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/4] api2: make email-from updatable

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/config.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
index 0b45f34c..3f060981 100644
--- a/src/api2/node/config.rs
+++ b/src/api2/node/config.rs
@@ -54,6 +54,8 @@ pub enum DeletableProperty {
     acmedomain4,
     /// Delete the http-proxy property.
     http_proxy,
+    /// Delete the email-from property.
+    email_from,
 }
 
 #[api(
@@ -110,6 +112,7 @@ pub fn update_node_config(
                 DeletableProperty::acmedomain3 => { config.acmedomain3 = None; },
                 DeletableProperty::acmedomain4 => { config.acmedomain4 = None; },
                 DeletableProperty::http_proxy => { config.http_proxy = None; },
+                DeletableProperty::email_from => { config.email_from = None; },
             }
         }
     }
@@ -121,6 +124,7 @@ pub fn update_node_config(
     if update.acmedomain3.is_some() { config.acmedomain3 = update.acmedomain3; }
     if update.acmedomain4.is_some() { config.acmedomain4 = update.acmedomain4; }
     if update.http_proxy.is_some() { config.http_proxy = update.http_proxy; }
+    if update.email_from.is_some() { config.email_from = update.email_from; }
 
     crate::config::node::save_config(&config)?;
 
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 3/4] server: use configured email-from for sending mail

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/server/email_notifications.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs
index 1e7650b0..0dc0cba5 100644
--- a/src/server/email_notifications.rs
+++ b/src/server/email_notifications.rs
@@ -235,6 +235,10 @@ fn send_job_status_mail(
     text: &str,
 ) -> Result<(), Error> {
 
+    let _lock = crate::config::node::lock()?;
+    let (config, _) = crate::config::node::config()?;
+    let from = config.email_from; 
+
     // Note: OX has serious problems displaying text mails,
     // so we include html as well
     let html = format!("<html><body><pre>\n{}\n<pre>", handlebars::html_escape(text));
@@ -248,7 +252,7 @@ fn send_job_status_mail(
         &subject,
         Some(&text),
         Some(&html),
-        None,
+        from.as_deref(),
         Some(&author),
     )?;
 
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup 3/4] server: use configured email-from for sending mail

[Dominik Csapak]

comment inline

On 12/14/21 13:44, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   src/server/email_notifications.rs | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs
> index 1e7650b0..0dc0cba5 100644
> --- a/src/server/email_notifications.rs
> +++ b/src/server/email_notifications.rs
> @@ -235,6 +235,10 @@ fn send_job_status_mail(
>       text: &str,
>   ) -> Result<(), Error> {
>   
> +    let _lock = crate::config::node::lock()?;
if you only read from the config, you do not need to lock it

we normally atomically replace the config on change, so a read
should always result in a valid config


> +    let (config, _) = crate::config::node::config()?;
> +    let from = config.email_from;
> +
>       // Note: OX has serious problems displaying text mails,
>       // so we include html as well
>       let html = format!("<html><body><pre>\n{}\n<pre>", handlebars::html_escape(text));
> @@ -248,7 +252,7 @@ fn send_job_status_mail(
>           &subject,
>           Some(&text),
>           Some(&html),
> -        None,
> +        from.as_deref(),
>           Some(&author),
>       )?;
>   

[pbs-devel] [PATCH proxmox-backup 4/4] ui: add new options tab under configuration

[Hannes Laimer]

... and add email-from + move http-proxy there

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/SystemConfiguration.js   | 25 +++++++++++++++++++++++--
 www/config/NodeOptionView.js |  9 +++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/www/SystemConfiguration.js b/www/SystemConfiguration.js
index 86371193..ac24bff9 100644
--- a/www/SystemConfiguration.js
+++ b/www/SystemConfiguration.js
@@ -63,9 +63,25 @@ Ext.define('PBS.SystemConfiguration', {
 		    title: gettext('Webauthn'),
 		    xtype: 'pbsWebauthnConfigView',
 		},
+	    ],
+	},
+	{
+	    title: gettext('Options'),
+	    itemId: 'options',
+	    xtype: 'panel',
+	    layout: {
+		type: 'vbox',
+		align: 'stretch',
+		multi: true,
+	    },
+	    defaults: {
+		collapsible: true,
+		animCollapse: false,
+		margin: '10 10 0 10',
+	    },
+	    items: [
 		{
-		    // FIXME: this is only a semi-OK place as long as there's only the http-proxy in there
-		    title: gettext('HTTP proxy'),
+		    title: gettext('General'),
 		    xtype: 'pbsNodeOptionView',
 		},
 	    ],
@@ -86,6 +102,11 @@ Ext.define('PBS.SystemConfiguration', {
 	Ext.Array.forEach(authentication.query(), function(item) {
 	    item.relayEvents(authentication, ['activate', 'deactivate', 'destroy']);
 	});
+
+	let options = me.getComponent('options');
+	Ext.Array.forEach(options.query(), function(item) {
+	    item.relayEvents(options, ['activate', 'deactivate', 'destroy']);
+	});
     },
 });
 
diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
index 55271a91..64f3d792 100644
--- a/www/config/NodeOptionView.js
+++ b/www/config/NodeOptionView.js
@@ -34,6 +34,15 @@ Ext.define('PBS.NodeOptionView', {
 	    deleteEmpty: true,
 	    onlineHelp: 'node_options_http_proxy',
 	},
+	{
+	    xtype: 'text',
+	    name: 'email-from',
+	    defaultValue: gettext('root@$hostname'),
+	    text: gettext('Email from address'),
+	    vtype: 'proxmoxMail',
+	    deleteEmpty: true,
+	},
+
     ],
 
     initComponent: function() {
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup 4/4] ui: add new options tab under configuration

[Dominik Csapak]

if i remember correctly, thomas did not like the
'configuration' -> 'options' approach (too generic)
or did that change @thomas?

(also the 'general' panel does make this not better imho)

On 12/14/21 13:44, Hannes Laimer wrote:
> ... and add email-from + move http-proxy there
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   www/SystemConfiguration.js   | 25 +++++++++++++++++++++++--
>   www/config/NodeOptionView.js |  9 +++++++++
>   2 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/www/SystemConfiguration.js b/www/SystemConfiguration.js
> index 86371193..ac24bff9 100644
> --- a/www/SystemConfiguration.js
> +++ b/www/SystemConfiguration.js
> @@ -63,9 +63,25 @@ Ext.define('PBS.SystemConfiguration', {
>   		    title: gettext('Webauthn'),
>   		    xtype: 'pbsWebauthnConfigView',
>   		},
> +	    ],
> +	},
> +	{
> +	    title: gettext('Options'),
> +	    itemId: 'options',
> +	    xtype: 'panel',
> +	    layout: {
> +		type: 'vbox',
> +		align: 'stretch',
> +		multi: true,
> +	    },
> +	    defaults: {
> +		collapsible: true,
> +		animCollapse: false,
> +		margin: '10 10 0 10',
> +	    },
> +	    items: [
>   		{
> -		    // FIXME: this is only a semi-OK place as long as there's only the http-proxy in there
> -		    title: gettext('HTTP proxy'),
> +		    title: gettext('General'),
>   		    xtype: 'pbsNodeOptionView',
>   		},
>   	    ],
> @@ -86,6 +102,11 @@ Ext.define('PBS.SystemConfiguration', {
>   	Ext.Array.forEach(authentication.query(), function(item) {
>   	    item.relayEvents(authentication, ['activate', 'deactivate', 'destroy']);
>   	});
> +
> +	let options = me.getComponent('options');
> +	Ext.Array.forEach(options.query(), function(item) {
> +	    item.relayEvents(options, ['activate', 'deactivate', 'destroy']);
> +	});
>       },
>   });
>   
> diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
> index 55271a91..64f3d792 100644
> --- a/www/config/NodeOptionView.js
> +++ b/www/config/NodeOptionView.js
> @@ -34,6 +34,15 @@ Ext.define('PBS.NodeOptionView', {
>   	    deleteEmpty: true,
>   	    onlineHelp: 'node_options_http_proxy',
>   	},
> +	{
> +	    xtype: 'text',
> +	    name: 'email-from',
> +	    defaultValue: gettext('root@$hostname'),
> +	    text: gettext('Email from address'),
> +	    vtype: 'proxmoxMail',
> +	    deleteEmpty: true,
> +	},
> +
>       ],
>   
>       initComponent: function() {