all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal