From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id B31C61FF17C
	for <inbox@lore.proxmox.com>; Wed,  2 Apr 2025 11:27:02 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id B5F8CD9F3;
	Wed,  2 Apr 2025 11:26:49 +0200 (CEST)
Message-ID: <11ced200-9ed0-4b9a-a387-cb355fcda3fe@proxmox.com>
Date: Wed, 2 Apr 2025 11:26:44 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: pve-devel@lists.proxmox.com
References: <20250328171340.885413-1-g.goller@proxmox.com>
 <20250328171340.885413-51-g.goller@proxmox.com>
Content-Language: en-US
From: Friedrich Weber <f.weber@proxmox.com>
In-Reply-To: <20250328171340.885413-51-g.goller@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.010 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH pve-manager 6/7] fabrics: Add main FabricView
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

Hi, two comments inline:

On 28/03/2025 18:13, Gabriel Goller wrote:
> TreeView that shows all the fabrics and nodes in a hierarchical
> structure. It also shows all the pending changes from the
> running-config.
> 
> We decided against including all the interfaces (as children of nodes)
> because otherwise the indentation would be too much. So to keep it
> simple, we removed the interface entries and also moved the protocol to
> the column (instead of having two root nodes for each protocol).
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> Co-authored-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  www/manager6/Makefile           |   1 +
>  www/manager6/dc/Config.js       |   8 +
>  www/manager6/sdn/FabricsView.js | 430 ++++++++++++++++++++++++++++++++
>  3 files changed, 439 insertions(+)
>  create mode 100644 www/manager6/sdn/FabricsView.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 7ed2839d9557..224b6079e833 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -303,6 +303,7 @@ JSSRC= 							\
>  	sdn/zones/SimpleEdit.js				\
>  	sdn/zones/VlanEdit.js				\
>  	sdn/zones/VxlanEdit.js				\
> +	sdn/FabricsView.js				\
>  	sdn/fabrics/Common.js				\
>  	sdn/fabrics/openfabric/FabricEdit.js		\
>  	sdn/fabrics/openfabric/NodeEdit.js		\
> diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
> index 74728c8320e9..68f7be8d6042 100644
> --- a/www/manager6/dc/Config.js
> +++ b/www/manager6/dc/Config.js
> @@ -229,6 +229,14 @@ Ext.define('PVE.dc.Config', {
>  		    hidden: true,
>  		    iconCls: 'fa fa-shield',
>  		    itemId: 'sdnfirewall',
> +		},
> +		{
> +		    xtype: 'pveSDNFabricView',
> +		    groups: ['sdn'],
> +		    title: gettext('Fabrics'),
> +		    hidden: true,
> +		    iconCls: 'fa fa-road',
> +		    itemId: 'sdnfabrics',
>  		});
>  	    }
>  
> diff --git a/www/manager6/sdn/FabricsView.js b/www/manager6/sdn/FabricsView.js
> new file mode 100644
> index 000000000000..0ef12defb1a8
> --- /dev/null
> +++ b/www/manager6/sdn/FabricsView.js
> @@ -0,0 +1,430 @@
> +const FABRIC_PANELS = {
> +    'openfabric': 'PVE.sdn.Fabric.OpenFabric.Fabric.Edit',
> +    'ospf': 'PVE.sdn.Fabric.Ospf.Fabric.Edit',
> +};
> +
> +const NODE_PANELS = {
> +    'openfabric': 'PVE.sdn.Fabric.OpenFabric.Node.Edit',
> +    'ospf': 'PVE.sdn.Fabric.Ospf.Node.Edit',
> +};
> +
> +Ext.define('PVE.sdn.Fabric.View', {
> +    extend: 'Ext.tree.Panel',
> +
> +    xtype: 'pveSDNFabricView',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    onlineHelp: 'pvesdn_config_fabrics',
> +
> +    columns: [
> +	{
> +	    xtype: 'treecolumn',
> +	    text: gettext('Name'),
> +	    dataIndex: 'name',
> +	    width: 200,
> +	    renderer: function(value, metaData, rec) {
> +		return PVE.Utils.render_sdn_pending(rec, value, 'name');
> +	    },
> +	},
> +	{
> +	    text: gettext('Protocol'),
> +	    dataIndex: 'protocol',
> +	    width: 100,
> +	    renderer: function(value, metaData, rec) {
> +		if (rec.data.type !== 'fabric') {
> +		    return "";
> +		}
> +
> +		return PVE.Utils.render_sdn_pending(rec, value, 'protocol');
> +	    },
> +	},
> +	{
> +	    text: gettext('Loopback IP'),
> +	    dataIndex: 'router_id',
> +	    width: 150,
> +	    renderer: function(value, metaData, rec) {
> +		if (rec.data.type === 'fabric') {
> +		    return PVE.Utils.render_sdn_pending(rec, rec.data.loopback_prefix, 'loopback_prefix');
> +		}
> +
> +		return PVE.Utils.render_sdn_pending(rec, value, 'router_id');
> +	    },
> +	},
> +	{
> +	    text: gettext('Action'),
> +	    xtype: 'actioncolumn',
> +	    dataIndex: 'text',
> +	    width: 100,
> +	    items: [
> +		{
> +		    handler: 'addActionTreeColumn',
> +		    getTip: (_v, _m, _rec) => gettext('Add'),
> +		    getClass: (_v, _m, { data }) => {
> +			if (data.type === 'fabric') {
> +			    return 'fa fa-plus-circle';
> +			}
> +
> +			return 'pmx-hidden';
> +		    },
> +		    isActionDisabled: (_v, _r, _c, _i, { data }) => data.type !== 'fabric',
> +		},
> +		{
> +		    tooltip: gettext('Edit'),
> +		    handler: 'editAction',
> +		    getClass: (_v, _m, { data }) => {
> +			// the fabric type (openfabric, ospf, etc.) cannot be edited
> +			if (data.type) {
> +			    return 'fa fa-pencil fa-fw';
> +			}
> +
> +			return 'pmx-hidden';
> +		    },
> +		    isActionDisabled: (_v, _r, _c, _i, { data }) => !data.type,
> +		},
> +		{
> +		    tooltip: gettext('Delete'),
> +		    handler: 'deleteAction',
> +		    getClass: (_v, _m, { data }) => {
> +			// the fabric type (openfabric, ospf, etc.) cannot be deleted
> +			if (data.type) {
> +			    return 'fa critical fa-trash-o';
> +			}
> +
> +			return 'pmx-hidden';
> +		    },
> +		    isActionDisabled: (_v, _r, _c, _i, { data }) => !data.type,
> +		},
> +	    ],
> +	},
> +	{
> +	    header: gettext('Interfaces'),
> +	    width: 100,
> +	    dataIndex: 'interface',
> +	    renderer: function(value, metaData, rec) {
> +		let interfaces = rec.data.pending?.interface || rec.data.interface || [];
> +
> +		let names = interfaces.map((iface) => {
> +		    let properties = Proxmox.Utils.parsePropertyString(iface);
> +		    return properties.name;
> +		});
> +
> +		names.sort();
> +		return names.join(", ");

Wouldn't hurt to htmlEncode here, to be on the safe side.

> +	    },
> +	},
> +	{
> +	    header: gettext('State'),
> +	    width: 100,
> +	    dataIndex: 'state',
> +	    renderer: function(value, metaData, rec) {
> +		return PVE.Utils.render_sdn_pending_state(rec, value);
> +	    },
> +	},
> +    ],
> +
> +    store: {
> +	sorters: ['name'],
> +    },
> +
> +    layout: 'fit',
> +    rootVisible: false,
> +    animate: false,
> +
> +
> +    initComponent: function() {
> +	let me = this;
> +
> +
> +	let add_button = new Proxmox.button.Button({
> +	    text: gettext('Add Node'),
> +	    handler: 'addActionTbar',
> +	    disabled: true,
> +	});
> +
> +	let set_add_button_status = function() {
> +	    let selection = me.view.getSelection();
> +
> +	    if (selection.length === 0) {
> +		return;
> +	    }
> +
> +	    add_button.setDisabled(selection[0].data.type !== 'fabric');
> +	};
> +
> +	Ext.apply(me, {
> +	    tbar: [
> +		{
> +		    text: gettext('Add Fabric'),
> +		    menu: [
> +			{
> +			    text: gettext('OpenFabric'),
> +			    handler: 'openAddOpenFabricWindow',
> +			},
> +			{
> +			    text: gettext('OSPF'),
> +			    handler: 'openAddOspfWindow',
> +			},
> +		    ],
> +		},
> +		add_button,
> +		{
> +		    xtype: 'proxmoxButton',
> +		    text: gettext('Reload'),
> +		    handler: 'reload',
> +		},
> +	    ],
> +	    listeners: {
> +		selectionchange: set_add_button_status,
> +	    },
> +	});
> +
> +	me.callParent();
> +    },
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	reload: function() {
> +	    let me = this;
> +
> +	    Proxmox.Utils.API2Request({
> +		url: `/cluster/sdn/fabrics/?pending=1`,
> +		method: 'GET',
> +		success: function(response, opts) {
> +		    let ospf = response.result.data.ospf;
> +		    let openfabric = response.result.data.openfabric;
> +
> +		    // add some metadata so we can merge the objects later and still know the protocol/type
> +		    ospf = ospf.map(x => {
> +			if (x.state && x.state === 'new') {
> +			    Object.assign(x, x.pending);
> +			}
> +
> +			if (x.ty === 'fabric') {
> +			    return Object.assign(x, { protocol: "ospf", name: x.area });
> +			} else if (x.ty === 'node') {
> +			    let id = x.node_id.split("_");
> +			    return Object.assign(x,
> +				{
> +				    protocol: "ospf",
> +				    node: id[1],
> +				    fabric: id[0],
> +				},
> +			    );
> +			} else {
> +			    return x;
> +			}
> +		    });
> +		    openfabric = openfabric.map(x => {
> +			if (x.state && x.state === 'new') {
> +			    Object.assign(x, x.pending);
> +			}
> +
> +			if (x.ty === 'fabric') {
> +			    return Object.assign(x, { protocol: "openfabric", name: x.fabric_id });
> +			} else if (x.ty === 'node') {
> +			    let id = x.node_id.split("_");
> +			    return Object.assign(x,
> +				{
> +				    protocol: "openfabric",
> +				    node: id[1],
> +				    fabric: id[0],
> +				},
> +			    );
> +			} else {
> +			    return x;
> +			}
> +		    });
> +
> +		    let allFabrics = openfabric.concat(ospf);
> +		    let fabrics = allFabrics.filter(e => e.ty === "fabric").map((fabric) => {
> +			if (!fabric.state || fabric.state !== 'deleted') {
> +			    fabric.children = allFabrics.filter(e => e.ty === "node")
> +				.filter((node) =>
> +				    node.fabric === fabric.name && node.protocol === fabric.protocol)
> +					.map((node) => {
> +					    Object.assign(node, {
> +						leaf: true,
> +						type: 'node',
> +						iconCls: 'fa fa-desktop x-fa-treepanel',
> +						name: node.node,
> +						_fabric: fabric.name,
> +					    });
> +
> +					    return node;
> +					});
> +			}
> +
> +			Object.assign(fabric, {
> +			    type: 'fabric',
> +			    protocol: fabric.protocol,
> +			    expanded: true,
> +			    name: fabric.fabric_id || fabric.area,
> +			    iconCls: 'fa fa-road x-fa-treepanel',
> +			});
> +
> +			return fabric;
> +		    });
> +
> +		    me.getView().setRootNode({
> +			name: '__root',
> +			expanded: true,
> +			children: fabrics,
> +		    });
> +		},
> +	    });
> +	},
> +
> +	getFabricEditPanel: function(type) {
> +	    return FABRIC_PANELS[type];
> +	},
> +
> +	getNodeEditPanel: function(type) {
> +	    return NODE_PANELS[type];
> +	},
> +
> +	addActionTreeColumn: function(_grid, _rI, _cI, _item, _e, rec) {
> +	    let me = this;
> +	    this.addAction(rec);
> +	},
> +
> +	addActionTbar: function() {
> +	    let me = this;
> +
> +	    let selection = me.view.getSelection();
> +
> +	    if (selection.length === 0) {
> +		return;
> +	    }
> +
> +	    if (selection[0].data.type === 'fabric') {
> +		me.addAction(selection[0]);
> +	    }
> +	},
> +
> +	addAction: function(rec) {
> +	    let me = this;
> +
> +	    let component = me.getNodeEditPanel(rec.data.protocol);
> +
> +	    if (!component) {
> +		console.warn(`unknown protocol ${rec.data.protocol}`);
> +		return;
> +	    }
> +
> +	    let extraRequestParams = {
> +		fabric: rec.data.name,
> +	    };
> +
> +	    let window = Ext.create(component, {
> +		autoShow: true,
> +		isCreate: true,
> +		autoLoad: false,
> +		extraRequestParams,
> +	    });
> +
> +	    window.on('destroy', () => me.reload());
> +	},
> +
> +	editAction: function(_grid, _rI, _cI, _item, _e, rec) {
> +	    let me = this;
> +
> +	    let component = '';
> +	    let url = '';
> +	    let autoLoad = true;
> +
> +	    if (rec.data.type === 'fabric') {
> +		component = me.getFabricEditPanel(rec.data.protocol);
> +		url = `/cluster/sdn/fabrics/${rec.data.protocol}/${rec.data.name}`;
> +	    } else if (rec.data.type === 'node') {
> +		component = me.getNodeEditPanel(rec.data.protocol);
> +		// no url, every request is done manually
> +		url = `/cluster/sdn/fabrics/${rec.data.protocol}/${rec.data._fabric}/node/${rec.data.node}`;
> +		autoLoad = false;
> +	    }
> +
> +	    if (!component) {
> +		console.warn(`unknown protocol ${rec.data.protocol} or unknown type ${rec.data.type}`);
> +		return;
> +	    }
> +
> +	    let window = Ext.create(component, {
> +		autoShow: true,
> +		autoLoad: autoLoad,
> +		isCreate: false,
> +		submitUrl: url,
> +		loadUrl: url,
> +		fabric: rec.data._fabric,
> +		node: rec.data.node,
> +	    });
> +
> +	    window.on('destroy', () => me.reload());
> +	},
> +
> +	deleteAction: function(table, rI, cI, item, e, { data }) {
> +	    let me = this;
> +	    let view = me.getView();
> +
> +	    Ext.Msg.show({
> +		title: gettext('Confirm'),
> +		icon: Ext.Msg.WARNING,
> +		message: Ext.String.format(gettext('Are you sure you want to remove the fabric {0}?'), `${data.name}`),

This is missing an htmlEncode.

> +		buttons: Ext.Msg.YESNO,
> +		defaultFocus: 'no',
> +		callback: function(btn) {
> +		    if (btn !== 'yes') {
> +			return;
> +		    }
> +
> +		    let url;
> +		    if (data.type === "node") {
> +			url = `/cluster/sdn/fabrics/${data.protocol}/${data._fabric}/node/${data.name}`;
> +		    } else if (data.type === "fabric") {
> +			url = `/cluster/sdn/fabrics/${data.protocol}/${data.name}`;
> +		    } else {
> +			console.warn("deleteAction: missing type");
> +		    }
> +
> +		    Proxmox.Utils.API2Request({
> +			url,
> +			method: 'DELETE',
> +			waitMsgTarget: view,
> +			failure: function(response, opts) {
> +			    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +			},
> +			callback: me.reload.bind(me),
> +		    });
> +		},
> +	    });
> +	},
> +
> +	openAddOpenFabricWindow: function() {
> +	    let me = this;
> +
> +	    let window = Ext.create('PVE.sdn.Fabric.OpenFabric.Fabric.Edit', {
> +		autoShow: true,
> +		autoLoad: false,
> +		isCreate: true,
> +	    });
> +
> +	    window.on('destroy', () => me.reload());
> +	},
> +
> +	openAddOspfWindow: function() {
> +	    let me = this;
> +
> +	    let window = Ext.create('PVE.sdn.Fabric.Ospf.Fabric.Edit', {
> +		autoShow: true,
> +		autoLoad: false,
> +		isCreate: true,
> +	    });
> +
> +	    window.on('destroy', () => me.reload());
> +	},
> +
> +	init: function(view) {
> +	    let me = this;
> +	    me.reload();
> +	},
> +    },
> +});



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