Re: [PATCH pve-manager v5 1/1] sdn: add dry-run diff view for sdn apply

[Gabriel Goller <g.goller@proxmox.com>]

On 12.03.2026 08:58, Hannes Laimer wrote:
> On 2026-03-10 13:07, Gabriel Goller wrote:
> > Introduce a new SdnDiffView modal that runs a dry-run and shows the frr
> > and ifupdown2 configuration changes which will be made when clicking
> > apply. Now the user knows which config options will be set without
> > needing to apply the config.
> > 
> > Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> > ---
> >  www/manager6/Makefile           |   1 +
> >  www/manager6/sdn/SdnDiffView.js | 149 ++++++++++++++++++++++++++++++++
> >  www/manager6/sdn/StatusView.js  |   8 ++
> >  3 files changed, 158 insertions(+)
> >  create mode 100644 www/manager6/sdn/SdnDiffView.js
> > 
> > diff --git a/www/manager6/sdn/SdnDiffView.js b/www/manager6/sdn/SdnDiffView.js
> > new file mode 100644
> > index 000000000000..e32e4cf896c4
> > --- /dev/null
> > +++ b/www/manager6/sdn/SdnDiffView.js
> > @@ -0,0 +1,149 @@
> > +Ext.define('PVE.sdn.SdnDiffView', {
> > +    extend: 'Ext.window.Window',
> > +
> > [snip]
> > +    loadDiff: async function () {
> > +        let me = this;
> > +
> > +        let req = await Proxmox.Async.api2({
> > +            url: `/cluster/sdn/dry-run`,
> > +            params: { node: me.node },
> > +            method: 'GET',
> > +        });
> > +
> > +        return req.result.data;
> > +    },
> > +
> > +    load: function () {
> > +        let me = this;
> > +
> > +        me.setLoading('fetching node diff');
> > +
> > +        me.loadDiff()
> > +            .catch(Proxmox.Utils.alertResponseFailure)
> 
> I think this should be .then(..).catch(..)

Ack.

> > +            .then((diff) => {
> > +                if (diff['frr-diff'] === null) {
> > +                    this.getViewModel().set('frr_diff', 'no changes');
> > +                } else {
> > +                    this.getViewModel().set('frr_diff', '<pre>' + diff['frr-diff'] + '</pre>');
> 
> we should probably `Ext.htmlEncode(diff['frr-diff'])`, this does
> container user config. The IS-IS controller domain field for example
> allows free-text ...
> 
> ```
> <h1>:P<img src="x"
> onerror="alert(document.cookie.match(/PVEAuthCookie=([^;]+)/)?.[1])"></h1>
> ```
> will work
> 
> > +                }
> > +                if (diff['interfaces-diff'] === null) {
> > +                    this.getViewModel().set('interfaces_diff', 'no changes');
> > +                } else {
> > +                    this.getViewModel().set(
> > +                        'interfaces_diff',
> > +                        '<pre>' + diff['interfaces-diff'] + '</pre>',
> 
> ... here as well

Makes sense, thanks for the review!


> > +                    );
> > +                }
> > +            })
> > +            .finally(() => {
> > +                me.setLoading(false);
> > +            });
> > +    },
> > +
> > +    getNodeSelector: function () {
> > +        let me = this;
> > +
> > +        return Ext.create('PVE.form.NodeSelector', {
> > +            xtype: 'pveNodeSelector',
> > +            reference: 'nodeselector',
> > +            fieldLabel: gettext('Node'),
> > +            padding: 10,
> > +            labelWidth: 120,
> > +            name: 'node',
> > +            allowBlank: false,
> > +            listeners: {
> > +                change: function (f, value) {
> > +                    me.node = value;
> > +                    me.load();
> > +                },
> > +            },
> > +            listConfig: {
> > +                columns: [
> > +                    {
> > +                        header: gettext('Node'),
> > +                        dataIndex: 'node',
> > +                        sortable: true,
> > +                        hideable: false,
> > +                        flex: 1,
> > +                    },
> > +                ],
> > +            },
> > +            store: {
> > +                fields: ['node'],
> > +                proxy: {
> > +                    type: 'proxmox',
> > +                    url: '/api2/json/nodes',
> > +                },
> > +                sorters: [
> > +                    {
> > +                        property: 'node',
> > +                        direction: 'ASC',
> > +                    },
> > +                ],
> > +            },
> > +        });
> > +    },
> > +
> > +    initComponent: function () {
> > +        let me = this;
> > +
> > +        me.nodeSelector = me.getNodeSelector();
> > +
> > +        me.items = [me.nodeSelector, ...me.items];
> > +
> > +        me.callParent();
> > +    },
> > +});
> > diff --git a/www/manager6/sdn/StatusView.js b/www/manager6/sdn/StatusView.js
> > index fbc712c6cf6b..fada50411e91 100644
> > --- a/www/manager6/sdn/StatusView.js
> > +++ b/www/manager6/sdn/StatusView.js
> > @@ -69,6 +69,14 @@ Ext.define(
> >                              });
> >                          },
> >                      },
> > +                    {
> > +                        text: gettext('Dry-Run'),
> > +                        handler: function () {
> > +                            Ext.create('PVE.sdn.SdnDiffView', {
> > +                                autoShow: true,
> > +                            });
> > +                        },
> > +                    },
> >                  ],
> >                  viewConfig: {
> >                      trackOver: false,
>