Hi, one minor comment inline:

On 28/03/2025 18:13, Gabriel Goller wrote:
> Implements a shared interface selector panel for openfabric and ospf fabrics.
> This GridPanel combines data from two sources: the node network interfaces
> (/nodes/<node>/network) and the fabrics section configuration, displaying
> a merged view of both.
> 
> It implements the following warning states:
> - When an interface has an IP address configured in /etc/network/interfaces,
>   we display a warning and disable the input field, prompting users to
>   configure addresses only via the fabrics interface
> - When addresses exist in both /etc/network/interfaces and
>   /etc/network/interfaces.d/sdn, we show a warning without disabling the 
> field,
>   allowing users to remove the SDN interface configuration while preserving
>   the underlying one
> 
> Signed-off-by: Gabriel Goller <g.gol...@proxmox.com>
> Co-authored-by: Stefan Hanreich <s.hanre...@proxmox.com>
> ---
>  www/manager6/Makefile              |   1 +
>  www/manager6/sdn/fabrics/Common.js | 285 +++++++++++++++++++++++++++++
>  2 files changed, 286 insertions(+)
>  create mode 100644 www/manager6/sdn/fabrics/Common.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index c94a5cdfbf70..7df96f58eb1f 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/fabrics/Common.js                           \
>       storage/ContentView.js                          \
>       storage/BackupView.js                           \
>       storage/Base.js                                 \
> diff --git a/www/manager6/sdn/fabrics/Common.js 
> b/www/manager6/sdn/fabrics/Common.js
> new file mode 100644
> index 000000000000..d71127d9c57f
> --- /dev/null
> +++ b/www/manager6/sdn/fabrics/Common.js
> @@ -0,0 +1,285 @@
> +Ext.define('PVE.sdn.Fabric.InterfacePanel', {
> +    extend: 'Ext.grid.Panel',
> +    mixins: ['Ext.form.field.Field'],
> +
> +    network_interfaces: undefined,
> +    parentClass: undefined,
> +
> +    selectionChange: function(_grid, selection) {
> +     let me = this;
> +     me.value = me.getSelection().map((rec) => {
> +         let submitValue = structuredClone(rec.data);
> +         delete submitValue.selected;
> +         delete submitValue.isDisabled;
> +         delete submitValue.statusIcon;
> +         delete submitValue.statusTooltip;
> +         delete submitValue.type;
> +         return PVE.Parser.printPropertyString(submitValue);
> +     });
> +     me.checkChange();
> +    },
> +
> +    getValue: function() {
> +        let me = this;
> +        return me.value ?? [];
> +    },
> +
> +    setValue: function(value) {
> +        let me = this;
> +
> +        value ??= [];
> +
> +        me.updateSelectedInterfaces(value);
> +
> +        return me.mixins.field.setValue.call(me, value);
> +    },
> +
> +    addInterfaces: function(fabricInterfaces) {
> +     let me = this;
> +     if (me.network_interfaces) {
> +         let nodeInterfaces = me.network_interfaces
> +             .map((elem) => {
> +                 const obj = {
> +                     name: elem.iface,
> +                     type: elem.type,
> +                     ip: elem.cidr,
> +                     ipv6: elem.cidr6,
> +                 };
> +                 return obj;
> +             });
> +
> +         if (fabricInterfaces) {
> +             // Map existing node interfaces with fabric data
> +             nodeInterfaces = nodeInterfaces.map(i => {
> +                 let elem = fabricInterfaces.find(j => j.name === i.name);
> +                 if (elem) {
> +                     if ((elem.ip && i.ip) || (elem.ipv6 && i.ipv6)) {
> +                         i.statusIcon = 'warning fa-warning';
> +                         i.statusTooltip = gettext('Interface already has an 
> address configured in /etc/network/interfaces');
> +                     } else if (i.ip || i.ipv6) {
> +                         i.statusIcon = 'warning fa-warning';
> +                         i.statusTooltip = gettext('Configure the ip-address 
> using the fabrics interface');
> +                         i.isDisabled = true;
> +                     }
> +                 }
> +                 return Object.assign(i, elem);
> +             });
> +
> +             // Add any fabric interface that doesn't exist in 
> node_interfaces
> +             for (const fabricIface of fabricInterfaces) {
> +                 if (!nodeInterfaces.some(nodeIface => nodeIface.name === 
> fabricIface.name)) {
> +                     nodeInterfaces.push({
> +                         name: fabricIface.name,
> +                         statusIcon: 'warning fa-warning',
> +                         statusTooltip: gettext('Interface not found on 
> node'),
> +                         ...fabricIface,
> +                     });
> +                 }
> +             }
> +             let store = me.getStore();
> +             store.setData(nodeInterfaces);
> +         } else {
> +             let store = me.getStore();
> +             store.setData(nodeInterfaces);
> +         }
> +     } else if (fabricInterfaces) {
> +         // We could not get the available interfaces of the node, so we 
> display the configured ones only.
> +         let interfaces = fabricInterfaces.map((elem) => {
> +             const obj = {
> +                 name: elem.name,
> +                 ...elem,
> +             };
> +             return obj;
> +         });
> +
> +         let store = me.getStore();
> +         store.setData(interfaces);
> +     } else {
> +         console.warn("no fabric_interfaces and cluster_interfaces 
> available!");
> +     }
> +    },
> +
> +    updateSelectedInterfaces: function(values) {
> +     let me = this;
> +     if (values) {
> +         let recs = [];
> +         let store = me.getStore();
> +
> +         for (const i of values) {
> +             let rec = store.getById(i.name);
> +             if (rec) {
> +                 recs.push(rec);
> +             }
> +         }
> +         me.suspendEvent('change');
> +         me.setSelection();
> +         me.setSelection(recs);
> +     } else {
> +         me.suspendEvent('change');
> +         me.setSelection();
> +     }
> +     me.resumeEvent('change');
> +    },
> +
> +    setNetworkInterfaces: function(network_interfaces) {
> +     this.network_interfaces = network_interfaces;
> +    },
> +
> +    getSubmitData: function() {
> +     let records = this.getSelection().map((record) => {
> +         let submitData = structuredClone(record.data);
> +         delete submitData.selected;
> +         delete submitData.isDisabled;
> +         delete submitData.statusIcon;
> +         delete submitData.statusTooltip;
> +         delete submitData.type;
> +
> +         // Delete any properties that are null or undefined
> +         Object.keys(submitData).forEach(function(key) {
> +             if (submitData[key] === null || submitData[key] === undefined 
> || submitData[key] === '') {
> +                 delete submitData[key];
> +             }
> +         });
> +
> +         return Proxmox.Utils.printPropertyString(submitData);
> +     });
> +     return {
> +         'interfaces': records,
> +     };
> +    },
> +
> +    controller: {
> +     onValueChange: function(field, value) {
> +         let me = this;
> +         let record = field.getWidgetRecord();
> +         let column = field.getWidgetColumn();
> +         if (record) {
> +             record.set(column.dataIndex, value);
> +             record.commit();
> +
> +             me.getView().checkChange();
> +             me.getView().selectionChange();
> +         }
> +     },
> +
> +     control: {
> +         'field': {
> +             change: 'onValueChange',
> +         },
> +     },
> +    },
> +
> +    selModel: {
> +     type: 'checkboxmodel',
> +     mode: 'SIMPLE',
> +    },
> +
> +    listeners: {
> +     selectionchange: function() {
> +         this.selectionChange(...arguments);
> +     },
> +    },
> +
> +    commonColumns: [
> +     {
> +         text: gettext('Status'),
> +         dataIndex: 'status',
> +         width: 30,
> +         renderer: function(value, metaData, record) {
> +             let icon = record.data.statusIcon || '';
> +             let tooltip = record.data.statusTooltip || '';
> +
> +             if (tooltip) {
> +                 metaData.tdAttr = 'data-qtip="' + Ext.htmlEncode(tooltip) + 
> '"';
> +             }

Tooltips need to be double-encoded [1], so one htmlEncode might be missing here.

[1] 
https://git.proxmox.com/?p=pve-manager.git;a=commit;h=f08f08a042cec0124f73199dcda0d8f882e14507

> +
> +             if (icon) {
> +                 return `<i class="fa ${icon}"></i>`;
> +             }
> +
> +             return value || '';
> +         },
> +
> +     },
> +     {
> +         text: gettext('Name'),
> +         dataIndex: 'name',
> +         flex: 2,
> +     },
> +     {
> +         text: gettext('Type'),
> +         dataIndex: 'type',
> +         flex: 1,
> +     },
> +     {
> +         text: gettext('IP'),
> +         xtype: 'widgetcolumn',
> +         dataIndex: 'ip',
> +         flex: 1,
> +
> +         widget: {
> +             xtype: 'proxmoxtextfield',
> +             isFormField: false,
> +             bind: {
> +                 disabled: '{record.isDisabled}',
> +             },
> +         },
> +     },
> +    ],
> +
> +    additionalColumns: [],
> +
> +    initComponent: function() {
> +     let me = this;
> +
> +     Ext.apply(me, {
> +         store: Ext.create("Ext.data.Store", {
> +             model: "Pve.sdn.Interface",
> +             sorters: {
> +                 property: 'name',
> +                 direction: 'ASC',
> +             },
> +         }),
> +         columns: me.commonColumns.concat(me.additionalColumns),
> +     });
> +
> +     me.callParent();
> +
> +     Proxmox.Utils.monStoreErrors(me, me.getStore(), true);
> +     me.initField();
> +    },
> +});
> +
> +
> +Ext.define('Pve.sdn.Fabric', {
> +    extend: 'Ext.data.Model',
> +    idProperty: 'name',
> +    fields: [
> +     'name',
> +     'type',
> +    ],
> +});
> +
> +Ext.define('Pve.sdn.Node', {
> +    extend: 'Ext.data.Model',
> +    idProperty: 'name',
> +    fields: [
> +     'name',
> +     'fabric',
> +     'type',
> +    ],
> +});
> +
> +Ext.define('Pve.sdn.Interface', {
> +    extend: 'Ext.data.Model',
> +    idProperty: 'name',
> +    fields: [
> +     'name',
> +     'ip',
> +     'ipv6',
> +     'passive',
> +     'hello_interval',
> +     'hello_multiplier',
> +     'csnp_interval',
> +    ],
> +});



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

Reply via email to