Re: [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

hey,

one note inline, just as an info though, no action required ^^

the patch works as advertised, so consider this:

Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>

thanks a lot for your contribution!

On Tue, Sep 29, 2026 at 04:23:31PM +0200, Michal Fox wrote:
> When adding an entry to an IPSet, the alias selector lists all aliases,
> including those that are already part of the IPSet. Selecting one of
> them only results in an "already exists" error from the API.
> 
> Pass the current entries of the IPSet to the add window and filter the
> matching aliases out of the selector. Aliases are stored in the IPSet
> with the same scoped reference the selector uses as value, so an exact
> comparison is sufficient.
note that this is only true for entries added after scoping was
introduced. entries added before (or by manually editing the firewall
config) may still reference aliases unscoped, so 'alias' and 'dc/alias'
can refer to the same thing as far as the API is concerned.

that said, I think exact comparison is fine here, since unscoped
references are an edge case. I do not think it would be worth it to 
implement the full resolving logic in the frontend, especially 
considering that we do not plan to keep supporting this type of 
reference forever.
> 
> Signed-off-by: Michal Fox <me@dualfroz.com>
> ---
>  www/manager6/form/IPRefSelector.js | 4 ++++
>  www/manager6/panel/IPSet.js        | 5 +++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/www/manager6/form/IPRefSelector.js b/www/manager6/form/IPRefSelector.js
> index 08fafd9b..7be09b6b 100644
> --- a/www/manager6/form/IPRefSelector.js
> +++ b/www/manager6/form/IPRefSelector.js
> @@ -8,6 +8,9 @@ Ext.define('PVE.form.IPRefSelector', {
>  
>      ref_type: undefined, // undefined = any [undefined, 'ipset' or 'alias']
>  
> +    // list of references that should not be offered, e.g. because they are already in use
> +    excludeRefs: [],
> +
>      valueField: 'scopedref',
>      displayField: 'ref',
>      notFoundIsValid: true,
> @@ -54,6 +57,7 @@ Ext.define('PVE.form.IPRefSelector', {
>                  property: 'ref',
>                  direction: 'ASC',
>              },
> +            filters: [(rec) => !me.excludeRefs.includes(rec.data.scopedref)],
>          });
>  
>          var columns = [];
> diff --git a/www/manager6/panel/IPSet.js b/www/manager6/panel/IPSet.js
> index 9e0203a0..d510ae6d 100644
> --- a/www/manager6/panel/IPSet.js
> +++ b/www/manager6/panel/IPSet.js
> @@ -187,6 +187,9 @@ Ext.define('PVE.IPSetCidrEdit', {
>  
>      cidr: undefined,
>  
> +    // entries already in the IPSet, not offered again when adding a new one
> +    existingCidrs: [],
> +
>      initComponent: function () {
>          var me = this;
>  
> @@ -211,6 +214,7 @@ Ext.define('PVE.IPSetCidrEdit', {
>                  xtype: 'pveIPRefSelector',
>                  name: 'cidr',
>                  ref_type: 'alias',
> +                excludeRefs: me.existingCidrs,
>                  autoSelect: false,
>                  editable: true,
>                  base_url: me.list_refs_url,
> @@ -359,6 +363,7 @@ Ext.define(
>                      var win = Ext.create('PVE.IPSetCidrEdit', {
>                          base_url: me.base_url,
>                          list_refs_url: me.list_refs_url,
> +                        existingCidrs: store.collect('cidr'),
>                      });
>                      win.show();
>                      win.on('destroy', reload);
> -- 
> 2.43.0
> 
> 
> 
>