[pve-devel] [PATCH manager v2 1/2] ui: fw: Close #2815: Add warning if fw is disabled

Dominic Jäger d.jaeger at proxmox.com
Wed Jun 24 11:32:31 CEST 2020


Currently people add firewall rules but forget to activate the firewall on
guest level. This commit adds a warning to the top bar of the firewall panel to
make them aware of this if necessary.

It avoids one O(n) lookup and might be sufficient to check only if some rule
exists and not for every rule if it is really enabled.

Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
---
v1->v2: Thank you for the feedback, Dominik!
 - The 10 top margin was intentional, but using 'auto' is better I think.
   It has in my tests avoided that the field randomly sticks to the top without
   breaking the rest of the spacing.
 - Message now depends on level: datacenter, node, guest. Using pveSelNode was
   the first solution that came to my mind and it didn't seem clearly inferior
   to adding an new "level" member (requirement to keep in sync with pveSelNode?)
   to the class.
 - Drop leftover 'dock' property

Not really convinced by the getLevel function but had no better idea
 www/manager6/grid/FirewallRules.js | 43 ++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index ec2d1c84..99f85fb6 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -483,8 +483,26 @@ Ext.define('PVE.FirewallRules', {
 	    throw "no list_refs_url specified";
 	}
 
+	let checkWarning = function () {
+	    Proxmox.Utils.API2Request({
+		url: me.base_url.replace('rules', 'options'),
+		method: 'GET',
+		failure: function (response) {
+		    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		},
+		success: function (response) {
+		    let warningRequired = me.store.getCount() != 0 && !response.result.data.enable;
+		    me.down('displayfield[name=fw-warning]').setVisible(warningRequired)
+		},
+	    });
+
+	};
+
 	var store = Ext.create('Ext.data.Store',{
-	    model: 'pve-fw-rule'
+	    model: 'pve-fw-rule',
+	    listeners: {
+		    'load': checkWarning,
+	    },
 	});
 
 	var reload = function() {
@@ -606,12 +624,33 @@ Ext.define('PVE.FirewallRules', {
 	    }
 	});
 
+	let getLevel = (id) => {
+	    let invalid = 'this';
+	    let level = /root/.test(id) ? 'datacenter'
+		: /node/.test(id) ? 'node'
+		: /qemu/.test(id) ? 'VM'
+		: /lxc/.test(id) ? 'container'
+		: invalid;
+	    if (level === invalid) { console.warn(`Finding level failed for ${id}`)};
+	    return level;
+	};
+	me.warningField = Ext.create('Ext.form.field.Display',{
+	    xtype: 'displayfield',
+	    userCls: 'pmx-hint',
+	    name: 'fw-warning',
+	    margin: 'auto 0 0 0', // Avoid field randomly sticking at top
+	    value: gettext(`Warning: Firewall still disabled at `
+		+ `${getLevel(me.pveSelNode.id)} level! `
+		+ `This can be changed in Firewall->Options.`),
+	    hidden: true,
+	});
+
 	var tbar = me.tbar_prefix ? [ me.tbar_prefix ] : [];
 	tbar.push(me.addBtn, me.copyBtn);
 	if (me.groupBtn) {
 	    tbar.push(me.groupBtn);
 	}
-	tbar.push(me.removeBtn, me.editBtn);
+	tbar.push(me.removeBtn, me.editBtn, me.warningField);
 
 	var render_errors = function(name, value, metaData, record) {
 	    var errors = record.data.errors;
-- 
2.20.1




More information about the pve-devel mailing list