public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager 0/2] ui: fw: allow selecting network interface for rules using combogrid
@ 2023-05-11  9:46 Christoph Heiss
  2023-05-11  9:46 ` [pve-devel] [PATCH manager 1/2] ui: fw: generalize `BridgeSelector` into network interface selector Christoph Heiss
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Christoph Heiss @ 2023-05-11  9:46 UTC (permalink / raw)
  To: pve-devel

For nodes, VMs and CTs we can show the user a list of available network
interfaces (as that information is available) when creating a new
firewall rule, much like it is already done in similar places.
Adds a lot of convenience when creating new firewall rules if they are
interface-specific, as you get a nice summary of the available ones and
can simply select it instead of typing it out each time.

The first patch refactors the `BridgeSelector` component a bit into a
new `NetworkInterfaceSelector`, is essence allowing it be used for any
type of network interfaces. No functional changes there.

The second patch contains the actual implementation, using the
`NetworkInterfaceSelector` from above for nodes and introducing a new
component (which is mostly based of the former) for VMs/CTs.
For datacenter rules, the simple textbox is kept.

pve-manager:

Christoph Heiss (2):
  ui: fw: generalize `BridgeSelector` into network interface selector
  ui: fw: allow selecting network interface for rules using combogrid

 www/manager6/Makefile                         |  3 +-
 www/manager6/form/BridgeSelector.js           | 71 -----------------
 www/manager6/form/NetworkInterfaceSelector.js | 79 +++++++++++++++++++
 .../form/VMNetworkInterfaceSelector.js        | 79 +++++++++++++++++++
 www/manager6/grid/FirewallRules.js            | 37 ++++++++-
 www/manager6/lxc/Config.js                    |  1 +
 www/manager6/lxc/Network.js                   |  3 +-
 www/manager6/qemu/Config.js                   |  1 +
 www/manager6/qemu/NetworkEdit.js              |  3 +-
 9 files changed, 199 insertions(+), 78 deletions(-)

--
2.39.2





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

* [pve-devel] [PATCH manager 1/2] ui: fw: generalize `BridgeSelector` into network interface selector
  2023-05-11  9:46 [pve-devel] [PATCH manager 0/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
@ 2023-05-11  9:46 ` Christoph Heiss
  2023-05-11  9:46 ` [pve-devel] [PATCH manager 2/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
  2023-09-26  7:37 ` [pve-devel] [PATCH manager 0/2] " Christoph Heiss
  2 siblings, 0 replies; 5+ messages in thread
From: Christoph Heiss @ 2023-05-11  9:46 UTC (permalink / raw)
  To: pve-devel

This makes it optional to specify a specific type of bridge/network and
renames the component to `NetworkInterfaceSelector`, to better fit it's
new role.

Allows reusing the component in other places, where the user should be
presented a choice of e.g. all available network interfaces on a node.

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 www/manager6/Makefile                         |  2 +-
 www/manager6/form/BridgeSelector.js           | 71 -----------------
 www/manager6/form/NetworkInterfaceSelector.js | 79 +++++++++++++++++++
 www/manager6/lxc/Network.js                   |  3 +-
 www/manager6/qemu/NetworkEdit.js              |  3 +-
 5 files changed, 84 insertions(+), 74 deletions(-)
 delete mode 100644 www/manager6/form/BridgeSelector.js
 create mode 100644 www/manager6/form/NetworkInterfaceSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 2b577c8e..a2f5116c 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -20,7 +20,6 @@ JSSRC= 							\
 	form/AgentFeatureSelector.js			\
 	form/BackupModeSelector.js			\
 	form/BandwidthSelector.js			\
-	form/BridgeSelector.js				\
 	form/BusTypeSelector.js				\
 	form/CPUModelSelector.js			\
 	form/CacheTypeSelector.js			\
@@ -47,6 +46,7 @@ JSSRC= 							\
 	form/MDevSelector.js				\
 	form/MemoryField.js				\
 	form/NetworkCardSelector.js			\
+	form/NetworkInterfaceSelector.js		\
 	form/NodeSelector.js				\
 	form/PCISelector.js				\
 	form/PermPathSelector.js			\
diff --git a/www/manager6/form/BridgeSelector.js b/www/manager6/form/BridgeSelector.js
deleted file mode 100644
index 350588cd..00000000
--- a/www/manager6/form/BridgeSelector.js
+++ /dev/null
@@ -1,71 +0,0 @@
-Ext.define('PVE.form.BridgeSelector', {
-    extend: 'Proxmox.form.ComboGrid',
-    alias: ['widget.PVE.form.BridgeSelector'],
-
-    bridgeType: 'any_bridge', // bridge, OVSBridge or any_bridge
-
-    store: {
-	fields: ['iface', 'active', 'type'],
-	filterOnLoad: true,
-	sorters: [
-	    {
-		property: 'iface',
-		direction: 'ASC',
-	    },
-	],
-    },
-    valueField: 'iface',
-    displayField: 'iface',
-    listConfig: {
-	columns: [
-	    {
-		header: gettext('Bridge'),
-		dataIndex: 'iface',
-		hideable: false,
-		width: 100,
-	    },
-	    {
-		header: gettext('Active'),
-		width: 60,
-		dataIndex: 'active',
-		renderer: Proxmox.Utils.format_boolean,
-	    },
-	    {
-		header: gettext('Comment'),
-		dataIndex: 'comments',
-		renderer: Ext.String.htmlEncode,
-		flex: 1,
-	    },
-	],
-    },
-
-    setNodename: function(nodename) {
-	var me = this;
-
-	if (!nodename || me.nodename === nodename) {
-	    return;
-	}
-
-	me.nodename = nodename;
-
-	me.store.setProxy({
-	    type: 'proxmox',
-	    url: '/api2/json/nodes/' + me.nodename + '/network?type=' +
-		me.bridgeType,
-	});
-
-	me.store.load();
-    },
-
-    initComponent: function() {
-	var me = this;
-
-	var nodename = me.nodename;
-	me.nodename = undefined;
-
-        me.callParent();
-
-	me.setNodename(nodename);
-    },
-});
-
diff --git a/www/manager6/form/NetworkInterfaceSelector.js b/www/manager6/form/NetworkInterfaceSelector.js
new file mode 100644
index 00000000..4c59b73e
--- /dev/null
+++ b/www/manager6/form/NetworkInterfaceSelector.js
@@ -0,0 +1,79 @@
+Ext.define('PVE.form.NetworkInterfaceSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: ['widget.PVE.form.NetworkInterfaceSelector'],
+
+    // Any of 'bridge, bond, eth, alias, vlan, OVSBridge, OVSBond, OVSPort, OVSIntPort, any_bridge'
+    // By default, all network interfaces are shown
+    networkType: undefined,
+
+    store: {
+	fields: ['iface', 'active', 'type'],
+	filterOnLoad: true,
+	sorters: [
+	    {
+		property: 'iface',
+		direction: 'ASC',
+	    },
+	],
+    },
+    valueField: 'iface',
+    displayField: 'iface',
+
+    setNodename: function(nodename) {
+	var me = this;
+
+	if (!nodename || me.nodename === nodename) {
+	    return;
+	}
+
+	me.nodename = nodename;
+
+	const type = me.networkType ? `?type=${me.networkType}` : '';
+
+	me.store.setProxy({
+	    type: 'proxmox',
+	    url: `/api2/json/nodes/${me.nodename}/network${type}`,
+	});
+
+	me.store.load();
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	var nodename = me.nodename;
+	me.nodename = undefined;
+
+	const isBridge = ['bridge', 'OVSBridge', 'any_bridge'].includes(me.networkType);
+
+	Ext.apply(me, {
+	    listConfig: {
+		columns: [
+		    {
+			header: isBridge ? gettext('Bridge') : gettext('Interface'),
+			dataIndex: 'iface',
+			hideable: false,
+			width: 100,
+		    },
+		    {
+			header: gettext('Active'),
+			width: 60,
+			dataIndex: 'active',
+			renderer: Proxmox.Utils.format_boolean,
+		    },
+		    {
+			header: gettext('Comment'),
+			dataIndex: 'comments',
+			renderer: Ext.String.htmlEncode,
+			flex: 1,
+		    },
+		],
+	    },
+	});
+
+	me.callParent();
+
+	me.setNodename(nodename);
+    },
+});
+
diff --git a/www/manager6/lxc/Network.js b/www/manager6/lxc/Network.js
index b2cd9410..2581ecd3 100644
--- a/www/manager6/lxc/Network.js
+++ b/www/manager6/lxc/Network.js
@@ -110,12 +110,13 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
 		emptyText: 'auto',
 	    },
 	    {
-		xtype: 'PVE.form.BridgeSelector',
+		xtype: 'PVE.form.NetworkInterfaceSelector',
 		name: 'bridge',
 		nodename: me.nodename,
 		fieldLabel: gettext('Bridge'),
 		value: cdata.bridge,
 		allowBlank: false,
+		networkType: 'any_bridge',
 	    },
 	    {
 		xtype: 'pveVlanField',
diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
index 4917eba5..b11a1ca0 100644
--- a/www/manager6/qemu/NetworkEdit.js
+++ b/www/manager6/qemu/NetworkEdit.js
@@ -76,12 +76,13 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 	me.column1 = [];
 	me.column2 = [];
 
-	me.bridgesel = Ext.create('PVE.form.BridgeSelector', {
+	me.bridgesel = Ext.create('PVE.form.NetworkInterfaceSelector', {
 	    name: 'bridge',
 	    fieldLabel: gettext('Bridge'),
 	    nodename: me.nodename,
 	    autoSelect: true,
 	    allowBlank: false,
+	    networkType: 'any_bridge',
 	});
 
 	me.column1 = [
-- 
2.39.2





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

* [pve-devel] [PATCH manager 2/2] ui: fw: allow selecting network interface for rules using combogrid
  2023-05-11  9:46 [pve-devel] [PATCH manager 0/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
  2023-05-11  9:46 ` [pve-devel] [PATCH manager 1/2] ui: fw: generalize `BridgeSelector` into network interface selector Christoph Heiss
@ 2023-05-11  9:46 ` Christoph Heiss
  2023-11-17 15:15   ` Wolfgang Bumiller
  2023-09-26  7:37 ` [pve-devel] [PATCH manager 0/2] " Christoph Heiss
  2 siblings, 1 reply; 5+ messages in thread
From: Christoph Heiss @ 2023-05-11  9:46 UTC (permalink / raw)
  To: pve-devel

For nodes, VMs and CTs we can show the user a list of available network
interfaces (as that information is available) when creating a new
firewall rule, much like it is already done in similar places.
Adds a lot of convenience when creating new firewall rules if they are
interface-specific, as you get a nice summary of the available ones and
can simply select it instead of typing it out each time.

Nodes can use the new `NetworkInterfaceSelector`, for VMs and CTs a new
component is needed, as the VM/CT config needs to be parsed
appropriately. It's mostly modeled after the `NetworkInterfaceSelector`
component and pretty straight-forward.
For datacenter rules, the simple textbox is kept.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Note: iptables(8) allows two wildcards for the interface, `!` and `+`.
For VMs and CTs this cannot be specified currently anyway, as the API
only allows /^net\d+$/. For nodes, since they accept any arbritrary
string as interface name, this possibility to specify a wildcard for the
interface gets essentially lost.

I guess we could still allow users to input any strings if they want -
is that something that should be possible (using the GUI)? IOW, do we
want to allow that?

 www/manager6/Makefile                         |  1 +
 .../form/VMNetworkInterfaceSelector.js        | 79 +++++++++++++++++++
 www/manager6/grid/FirewallRules.js            | 37 ++++++++-
 www/manager6/lxc/Config.js                    |  1 +
 www/manager6/qemu/Config.js                   |  1 +
 5 files changed, 115 insertions(+), 4 deletions(-)
 create mode 100644 www/manager6/form/VMNetworkInterfaceSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index a2f5116c..57ba331b 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -71,6 +71,7 @@ JSSRC= 							\
 	form/UserSelector.js				\
 	form/VLanField.js				\
 	form/VMCPUFlagSelector.js			\
+	form/VMNetworkInterfaceSelector.js		\
 	form/VMSelector.js				\
 	form/VNCKeyboardSelector.js			\
 	form/ViewSelector.js				\
diff --git a/www/manager6/form/VMNetworkInterfaceSelector.js b/www/manager6/form/VMNetworkInterfaceSelector.js
new file mode 100644
index 00000000..fbe631ba
--- /dev/null
+++ b/www/manager6/form/VMNetworkInterfaceSelector.js
@@ -0,0 +1,79 @@
+Ext.define('PVE.form.VMNetworkInterfaceSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.PVE.form.VMNetworkInterfaceSelector',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    cbindData: (initialConfig) => ({
+	isQemu: initialConfig.pveSelNode.data.type === 'qemu',
+    }),
+
+    displayField: 'id',
+
+    store: {
+	fields: ['id', 'name', 'bridge', 'ip'],
+	filterOnLoad: true,
+	sorters: {
+	    property: 'id',
+	    direction: 'ASC',
+	},
+    },
+
+    listConfig: {
+	cbind: {},
+	columns: [
+	    {
+		header: 'ID',
+		dataIndex: 'id',
+		hideable: false,
+		width: 80,
+	    },
+	    {
+		header: gettext('Name'),
+		dataIndex: 'name',
+		flex: 1,
+		cbind: {
+		    hidden: '{isQemu}',
+		},
+	    },
+	    {
+		header: gettext('Bridge'),
+		dataIndex: 'bridge',
+		flex: 1,
+	    },
+	    {
+		header: gettext('IP address'),
+		dataIndex: 'ip',
+		flex: 1,
+		cbind: {
+		    hidden: '{isQemu}',
+		},
+	    },
+	],
+    },
+
+    initComponent: function() {
+	const { node: nodename, type, vmid } = this.pveSelNode.data;
+
+	Proxmox.Utils.API2Request({
+	    url: `/nodes/${nodename}/${type}/${vmid}/config`,
+	    method: 'GET',
+	    success: ({ result: { data } }) => {
+		let networks = [];
+		for (const [id, value] of Object.entries(data)) {
+		    if (id.match(/^net\d+/)) {
+			const parsed = type === 'lxc'
+			    ? PVE.Parser.parseLxcNetwork(value)
+			    : PVE.Parser.parseQemuNetwork(id, value);
+
+			networks.push({ ...parsed, id });
+		    }
+		}
+
+		this.store.loadData(networks);
+	    },
+	});
+
+	this.callParent();
+    },
+});
+
diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
index 5777c7f4..9085bd64 100644
--- a/www/manager6/grid/FirewallRules.js
+++ b/www/manager6/grid/FirewallRules.js
@@ -153,6 +153,7 @@ Ext.define('PVE.FirewallRulePanel', {
     allow_iface: false,

     list_refs_url: undefined,
+    pveSelNode: undefined,

     onGetValues: function(values) {
 	var me = this;
@@ -206,13 +207,35 @@ Ext.define('PVE.FirewallRulePanel', {
         ];

 	if (me.allow_iface) {
-	    me.column1.push({
-		xtype: 'proxmoxtextfield',
+	    const commonFields = {
 		name: 'iface',
 		deleteEmpty: !me.isCreate,
-		value: '',
 		fieldLabel: gettext('Interface'),
-	    });
+		allowBlank: true,
+		autoSelect: false,
+	    };
+
+	    if (me.pveSelNode?.data.type === 'node') {
+		me.column1.push({
+		    ...commonFields,
+		    xtype: 'PVE.form.NetworkInterfaceSelector',
+		    nodename: me.pveSelNode.data.node,
+		});
+	    } else if (me.pveSelNode?.data.type) {
+		// qemu and lxc
+		me.column1.push({
+		    ...commonFields,
+		    xtype: 'PVE.form.VMNetworkInterfaceSelector',
+		    pveSelNode: me.pveSelNode,
+		});
+	    } else {
+		// datacenter
+		me.column1.push({
+		    ...commonFields,
+		    xtype: 'proxmoxtextfield',
+		    value: '',
+		});
+	    }
 	} else {
 	    me.column1.push({
 		xtype: 'displayfield',
@@ -386,6 +409,7 @@ Ext.define('PVE.FirewallRuleEdit', {

     base_url: undefined,
     list_refs_url: undefined,
+    pveSelNode: undefined,

     allow_iface: false,

@@ -414,6 +438,7 @@ Ext.define('PVE.FirewallRuleEdit', {
 	    list_refs_url: me.list_refs_url,
 	    allow_iface: me.allow_iface,
 	    rule_pos: me.rule_pos,
+	    pveSelNode: me.pveSelNode,
 	});

 	Ext.apply(me, {
@@ -546,6 +571,7 @@ Ext.define('PVE.FirewallRules', {

     base_url: undefined,
     list_refs_url: undefined,
+    pveSelNode: undefined,

     addBtn: undefined,
     removeBtn: undefined,
@@ -671,6 +697,7 @@ Ext.define('PVE.FirewallRules', {
 		base_url: me.base_url,
 		list_refs_url: me.list_refs_url,
 		rule_pos: rec.data.pos,
+		pveSelNode: me.pveSelNode,
 	    });

 	    win.show();
@@ -692,6 +719,7 @@ Ext.define('PVE.FirewallRules', {
 		    allow_iface: me.allow_iface,
 		    base_url: me.base_url,
 		    list_refs_url: me.list_refs_url,
+		    pveSelNode: me.pveSelNode,
 		});
 		win.on('destroy', reload);
 		win.show();
@@ -713,6 +741,7 @@ Ext.define('PVE.FirewallRules', {
 		base_url: me.base_url,
 		list_refs_url: me.list_refs_url,
 		rec: rec,
+		pveSelNode: me.pveSelNode,
 	    });
 	    win.show();
 	    win.on('destroy', reload);
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 23c17d2e..1bd82413 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -314,6 +314,7 @@ Ext.define('PVE.lxc.Config', {
 		    base_url: base_url + '/firewall/rules',
 		    list_refs_url: base_url + '/firewall/refs',
 		    itemId: 'firewall',
+		    pveSelNode: me.pveSelNode,
 		},
 		{
 		    xtype: 'pveFirewallOptions',
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 94c540c5..dfe2c56f 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -349,6 +349,7 @@ Ext.define('PVE.qemu.Config', {
 		    base_url: base_url + '/firewall/rules',
 		    list_refs_url: base_url + '/firewall/refs',
 		    itemId: 'firewall',
+		    pveSelNode: me.pveSelNode,
 		},
 		{
 		    xtype: 'pveFirewallOptions',
--
2.39.2





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

* Re: [pve-devel] [PATCH manager 0/2] ui: fw: allow selecting network interface for rules using combogrid
  2023-05-11  9:46 [pve-devel] [PATCH manager 0/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
  2023-05-11  9:46 ` [pve-devel] [PATCH manager 1/2] ui: fw: generalize `BridgeSelector` into network interface selector Christoph Heiss
  2023-05-11  9:46 ` [pve-devel] [PATCH manager 2/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
@ 2023-09-26  7:37 ` Christoph Heiss
  2 siblings, 0 replies; 5+ messages in thread
From: Christoph Heiss @ 2023-09-26  7:37 UTC (permalink / raw)
  To: Proxmox VE development discussion


Ping.

While it does not apply cleanly on current master anymore, I'd like to
collect some general feedback on the approach before rebasing +
resending w/o any actual changes.

On Thu, May 11, 2023 at 11:46:18AM +0200, Christoph Heiss wrote:
>
> For nodes, VMs and CTs we can show the user a list of available network
> interfaces (as that information is available) when creating a new
> firewall rule, much like it is already done in similar places.
> Adds a lot of convenience when creating new firewall rules if they are
> interface-specific, as you get a nice summary of the available ones and
> can simply select it instead of typing it out each time.
>
> The first patch refactors the `BridgeSelector` component a bit into a
> new `NetworkInterfaceSelector`, is essence allowing it be used for any
> type of network interfaces. No functional changes there.
>
> The second patch contains the actual implementation, using the
> `NetworkInterfaceSelector` from above for nodes and introducing a new
> component (which is mostly based of the former) for VMs/CTs.
> For datacenter rules, the simple textbox is kept.
>
> pve-manager:
>
> Christoph Heiss (2):
>   ui: fw: generalize `BridgeSelector` into network interface selector
>   ui: fw: allow selecting network interface for rules using combogrid
>
>  www/manager6/Makefile                         |  3 +-
>  www/manager6/form/BridgeSelector.js           | 71 -----------------
>  www/manager6/form/NetworkInterfaceSelector.js | 79 +++++++++++++++++++
>  .../form/VMNetworkInterfaceSelector.js        | 79 +++++++++++++++++++
>  www/manager6/grid/FirewallRules.js            | 37 ++++++++-
>  www/manager6/lxc/Config.js                    |  1 +
>  www/manager6/lxc/Network.js                   |  3 +-
>  www/manager6/qemu/Config.js                   |  1 +
>  www/manager6/qemu/NetworkEdit.js              |  3 +-
>  9 files changed, 199 insertions(+), 78 deletions(-)
>
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>




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

* Re: [pve-devel] [PATCH manager 2/2] ui: fw: allow selecting network interface for rules using combogrid
  2023-05-11  9:46 ` [pve-devel] [PATCH manager 2/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
@ 2023-11-17 15:15   ` Wolfgang Bumiller
  0 siblings, 0 replies; 5+ messages in thread
From: Wolfgang Bumiller @ 2023-11-17 15:15 UTC (permalink / raw)
  To: Christoph Heiss; +Cc: pve-devel

just some thoughts from my side:

On Thu, May 11, 2023 at 11:46:20AM +0200, Christoph Heiss wrote:
> For nodes, VMs and CTs we can show the user a list of available network
> interfaces (as that information is available) when creating a new
> firewall rule, much like it is already done in similar places.
> Adds a lot of convenience when creating new firewall rules if they are
> interface-specific, as you get a nice summary of the available ones and
> can simply select it instead of typing it out each time.
> 
> Nodes can use the new `NetworkInterfaceSelector`, for VMs and CTs a new
> component is needed, as the VM/CT config needs to be parsed
> appropriately. It's mostly modeled after the `NetworkInterfaceSelector`
> component and pretty straight-forward.
> For datacenter rules, the simple textbox is kept.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Note: iptables(8) allows two wildcards for the interface, `!` and `+`.
> For VMs and CTs this cannot be specified currently anyway, as the API
> only allows /^net\d+$/. For nodes, since they accept any arbritrary
> string as interface name, this possibility to specify a wildcard for the
> interface gets essentially lost.
> 
> I guess we could still allow users to input any strings if they want -
> is that something that should be possible (using the GUI)? IOW, do we
> want to allow that?



> 
>  www/manager6/Makefile                         |  1 +
>  .../form/VMNetworkInterfaceSelector.js        | 79 +++++++++++++++++++
>  www/manager6/grid/FirewallRules.js            | 37 ++++++++-
>  www/manager6/lxc/Config.js                    |  1 +
>  www/manager6/qemu/Config.js                   |  1 +
>  5 files changed, 115 insertions(+), 4 deletions(-)
>  create mode 100644 www/manager6/form/VMNetworkInterfaceSelector.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index a2f5116c..57ba331b 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -71,6 +71,7 @@ JSSRC= 							\
>  	form/UserSelector.js				\
>  	form/VLanField.js				\
>  	form/VMCPUFlagSelector.js			\
> +	form/VMNetworkInterfaceSelector.js		\
>  	form/VMSelector.js				\
>  	form/VNCKeyboardSelector.js			\
>  	form/ViewSelector.js				\
> diff --git a/www/manager6/form/VMNetworkInterfaceSelector.js b/www/manager6/form/VMNetworkInterfaceSelector.js
> new file mode 100644
> index 00000000..fbe631ba
> --- /dev/null
> +++ b/www/manager6/form/VMNetworkInterfaceSelector.js
> @@ -0,0 +1,79 @@
> +Ext.define('PVE.form.VMNetworkInterfaceSelector', {
> +    extend: 'Proxmox.form.ComboGrid',
> +    alias: 'widget.PVE.form.VMNetworkInterfaceSelector',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    cbindData: (initialConfig) => ({
> +	isQemu: initialConfig.pveSelNode.data.type === 'qemu',
> +    }),
> +
> +    displayField: 'id',
> +
> +    store: {
> +	fields: ['id', 'name', 'bridge', 'ip'],

Not a fan of only including the 'ip' field without also including the
'ip6' field. And not sure about the formatting with both included :-)
(They can also be "manual" and "dhcp", and ip6 can additionally be
"auto", so it might look weird, but 🤷)

In patch 1 the NetworkInterfaceSelector has different fields
('iface', 'active', 'type')

> +	filterOnLoad: true,
> +	sorters: {
> +	    property: 'id',
> +	    direction: 'ASC',
> +	},
> +    },
> +
> +    listConfig: {
> +	cbind: {},
> +	columns: [
> +	    {
> +		header: 'ID',
> +		dataIndex: 'id',
> +		hideable: false,
> +		width: 80,
> +	    },
> +	    {
> +		header: gettext('Name'),
> +		dataIndex: 'name',
> +		flex: 1,
> +		cbind: {
> +		    hidden: '{isQemu}',
> +		},
> +	    },
> +	    {
> +		header: gettext('Bridge'),
> +		dataIndex: 'bridge',
> +		flex: 1,
> +	    },
> +	    {
> +		header: gettext('IP address'),
> +		dataIndex: 'ip',
> +		flex: 1,
> +		cbind: {
> +		    hidden: '{isQemu}',
> +		},
> +	    },
> +	],
> +    },
> +
> +    initComponent: function() {
> +	const { node: nodename, type, vmid } = this.pveSelNode.data;
> +
> +	Proxmox.Utils.API2Request({
> +	    url: `/nodes/${nodename}/${type}/${vmid}/config`,
> +	    method: 'GET',
> +	    success: ({ result: { data } }) => {
> +		let networks = [];
> +		for (const [id, value] of Object.entries(data)) {
> +		    if (id.match(/^net\d+/)) {
> +			const parsed = type === 'lxc'
> +			    ? PVE.Parser.parseLxcNetwork(value)
> +			    : PVE.Parser.parseQemuNetwork(id, value);
> +
> +			networks.push({ ...parsed, id });
> +		    }
> +		}
> +
> +		this.store.loadData(networks);
> +	    },
> +	});
> +
> +	this.callParent();
> +    },
> +});
> +
> diff --git a/www/manager6/grid/FirewallRules.js b/www/manager6/grid/FirewallRules.js
> index 5777c7f4..9085bd64 100644
> --- a/www/manager6/grid/FirewallRules.js
> +++ b/www/manager6/grid/FirewallRules.js
> @@ -153,6 +153,7 @@ Ext.define('PVE.FirewallRulePanel', {
>      allow_iface: false,
> 
>      list_refs_url: undefined,
> +    pveSelNode: undefined,
> 
>      onGetValues: function(values) {
>  	var me = this;
> @@ -206,13 +207,35 @@ Ext.define('PVE.FirewallRulePanel', {
>          ];
> 
>  	if (me.allow_iface) {
> -	    me.column1.push({
> -		xtype: 'proxmoxtextfield',
> +	    const commonFields = {
>  		name: 'iface',
>  		deleteEmpty: !me.isCreate,
> -		value: '',
>  		fieldLabel: gettext('Interface'),
> -	    });
> +		allowBlank: true,
> +		autoSelect: false,
> +	    };
> +
> +	    if (me.pveSelNode?.data.type === 'node') {
> +		me.column1.push({
> +		    ...commonFields,
> +		    xtype: 'PVE.form.NetworkInterfaceSelector',
> +		    nodename: me.pveSelNode.data.node,
> +		});
> +	    } else if (me.pveSelNode?.data.type) {
> +		// qemu and lxc
> +		me.column1.push({
> +		    ...commonFields,
> +		    xtype: 'PVE.form.VMNetworkInterfaceSelector',

^ The store here AFAICT has different fields.
Non-VM one has 'iface', 'active', type', the VM one has 'id', 'name',
'bridge', 'ip'.
It may just show how little I deal with our extjs code, but that seems
like it shouldn't quite fit here 🤷




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

end of thread, other threads:[~2023-11-17 15:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-11  9:46 [pve-devel] [PATCH manager 0/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
2023-05-11  9:46 ` [pve-devel] [PATCH manager 1/2] ui: fw: generalize `BridgeSelector` into network interface selector Christoph Heiss
2023-05-11  9:46 ` [pve-devel] [PATCH manager 2/2] ui: fw: allow selecting network interface for rules using combogrid Christoph Heiss
2023-11-17 15:15   ` Wolfgang Bumiller
2023-09-26  7:37 ` [pve-devel] [PATCH manager 0/2] " Christoph Heiss

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