public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added
@ 2026-09-29 14:23 Michal Fox
  2026-09-30  9:50 ` Arthur Bied-Charreton
  2026-09-30 10:27 ` applied: " Dominik Csapak
  0 siblings, 2 replies; 3+ messages in thread
From: Michal Fox @ 2026-09-29 14:23 UTC (permalink / raw)
  To: pve-devel

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.

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




^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added
  2026-09-29 14:23 [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added Michal Fox
@ 2026-09-30  9:50 ` Arthur Bied-Charreton
  2026-09-30 10:27 ` applied: " Dominik Csapak
  1 sibling, 0 replies; 3+ messages in thread
From: Arthur Bied-Charreton @ 2026-09-30  9:50 UTC (permalink / raw)
  To: Michal Fox; +Cc: pve-devel

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




^ permalink raw reply	[flat|nested] 3+ messages in thread

* applied: [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added
  2026-09-29 14:23 [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added Michal Fox
  2026-09-30  9:50 ` Arthur Bied-Charreton
@ 2026-09-30 10:27 ` Dominik Csapak
  1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2026-09-30 10:27 UTC (permalink / raw)
  To: pve-devel, Michal Fox

On Tue, 29 Sep 2026 16:23:31 +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.
> 
> [...]

Applied, thanks!

[1/1] fix #7108: ui: ipset: do not offer aliases that are already added
      commit: 8efee570fbe9b99df40daf6ab83d77db295379cb




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-30 10:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-29 14:23 [PATCH manager] fix #7108: ui: ipset: do not offer aliases that are already added Michal Fox
2026-09-30  9:50 ` Arthur Bied-Charreton
2026-09-30 10:27 ` applied: " Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal