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

[Hannes Laimer <h.laimer@proxmox.com>]

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/Makefile b/www/manager6/Makefile
> index 4558d53e54be..da602523b27a 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -286,6 +286,7 @@ JSSRC= 							\
>  	sdn/ControllerView.js				\
>  	sdn/Status.js					\
>  	sdn/StatusView.js				\
> +	sdn/SdnDiffView.js				\
>  	sdn/VnetEdit.js					\
>  	sdn/VnetView.js					\
>  	sdn/VnetACLView.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',
> +
> +    width: 800,
> +    height: 900,
> +
> +    scrollable: true,
> +    modal: true,
> +    title: gettext('Pending SDN configuration changes'),
> +
> +    node: undefined,
> +
> +    viewModel: {
> +        data: {
> +            frr_diff: undefined,
> +            interfaces_diff: undefined,
> +        },
> +    },
> +
> +    items: [
> +        {
> +            xtype: 'panel',
> +            title: gettext('FRR Config'),
> +            bodyPadding: 10,
> +            items: [
> +                {
> +                    xtype: 'displayfield',
> +                    bind: {
> +                        value: '{frr_diff}',
> +                    },
> +                },
> +            ],
> +        },
> +        {
> +            xtype: 'panel',
> +            title: gettext('Interfaces Config'),
> +            bodyPadding: 10,
> +            items: [
> +                {
> +                    xtype: 'displayfield',
> +                    bind: {
> +                        value: '{interfaces_diff}',
> +                    },
> +                },
> +            ],
> +        },
> +    ],
> +    buttons: [
> +        {
> +            handler: function () {
> +                this.up('window').close();
> +            },
> +            text: gettext('Close'),
> +        },
> +    ],
> +
> +    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(..)

> +            .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

> +                    );
> +                }
> +            })
> +            .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,