From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 044531FF0A7 for ; Tue, 18 Aug 2026 15:34:50 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7904F215F1; Tue, 18 Aug 2026 15:34:20 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH firewall/manager v2 0/9] allow updating references when renaming/deleting firewall objects Date: Tue, 18 Aug 2026 15:34:04 +0200 Message-ID: <20260818133413.450776-1-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.838 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: MQ2SCTDIWVJKWBJ3K3EUCL2AATGISO4U X-Message-ID-Hash: MQ2SCTDIWVJKWBJ3K3EUCL2AATGISO4U X-MailFrom: abied-charreton@jett.proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Renaming or deleting a firewall object (IPSet or alias) that is still referenced currently leaves dangling references behind. The firewall then fails to parse the affected rules, which become inactive without any error visible from the UI. This is especially dangerous on renames, where a user might reasonably expect references to follow the rename, and could walk away believing a set of rules is active while it has been effectively disabled. This series introduces opt-in functionality allowing the related firewall endpoints to keep references consistent. - rename (POST /ipset/, PUT /aliases/{name}) gains an 'update-references' option to rewrite all references to the new name. - delete (DELETE /ipset/{name}, DELETE /aliases/{name}) gains a 'delete-references' option to drop the referencing rules along with the object. In the UI, 'update-references' is represented as a segmented button in the edit panels allowing to set 'no', 'yes' and 'force' (see below). 'delete-references' is a check box in the remove confirm dialog. The reference rewriting is centralized in a new helper that scans rules (also in security groups) and IPSet members. When called for a cluster object, it also updates references in downstream configs (guest, host, vnet) across the cluster, since those may reference cluster-level objects. On a cluster rename, the new object is persisted before its references are rewritten, and the old object is removed only afterwards. At no point does an on-disk state contain a reference that is not defined, which would otherwise make a concurrent firewall compilation drop the affected rule. The trade-off is that an interrupted cluster rename leaves both the old and new object defined, possibly with references to both in downstream configs. In order to have something standing between the admin and manually fixing references across a whole cluster's worth of configs in case of mid-rename failure, the rename can be finished by re-issuing it. The 'update-references' option is an enum, 'no' for opting out, 'yes' for opting in, and 'force' for forcing the rename even if an object already exists with the target name, which is the state a partial cluster rename would leave the config in. Only cluster objects fan out, host and vnet configs have no IPSet/alias section of their own, and SDN-generated (sdn/) IPSets are regenerated from the SDN config and never persisted, so neither is touched. Future work: - As noted by Stefan in the v1 review [0], IPSets are auto-generated for SDN objects. Those IPSets can also be referenced in user-managed configs and would therefore benefit from a similar feature for the SDN object edit/delete paths. It should be relatively straightforward to extend the shared helper (update_refs, commit 1 in pve-firewall) to also handle SDN references. Changes since v1 (thanks @Stefan): - Fix timing issue where on rename, the cluster fan-out would update downstream references to use the new name before persisting it in the cluster config. This would cause concurrent firewall compilations to fail to parse some rules and drop them. [2] - Scan vnet configs for references as well [1] - Generalize the cluster fan-out logic for renaming/deleting object references, and move all shared functionality into the Helpers module. [2] - Make the 'update-references' option an enum to allow retrying a partial rename caused by a failure during cluster fan-out. - Rename the filter_map helper to map_items. [1] - Use POD for doc comments. [1] [0] https://lore.proxmox.com/all/e7348b76-a215-4189-ade9-a7c5bbbd26cf@proxmox.com/ [1] https://lore.proxmox.com/all/c161fc57-1d8e-4340-9d2c-6621d1c39af8@proxmox.com/ [2] https://lore.proxmox.com/all/055351eb-f575-46c3-ac37-01925306c449@proxmox.com/ pve-firewall: Arthur Bied-Charreton (6): api: helpers: add helper to update firewall object references api: ipset: add option to update references on edit api: ipset: add option to GC references on delete api: aliases: add option to update references on edit api: aliases: add option to GC references on delete firewall: tests: add tests for object reference update logic src/PVE/API2/Firewall/Aliases.pm | 51 +++++++- src/PVE/API2/Firewall/IPSet.pm | 71 +++++++++-- src/PVE/Firewall/Helpers.pm | 195 +++++++++++++++++++++++++++++++ test/Makefile | 1 + test/referenceupdatetests.pl | 178 ++++++++++++++++++++++++++++ 5 files changed, 482 insertions(+), 14 deletions(-) create mode 100755 test/referenceupdatetests.pl pve-manager: Arthur Bied-Charreton (3): ui: firewall: add common widgets for deleting and updating references ui: firewall: ipset: add controls to update/delete references on edit ui: firewall: aliases: add controls to update/delete references on edit www/manager6/Makefile | 1 + www/manager6/grid/FirewallAliases.js | 74 +++++++++++----- www/manager6/grid/FirewallObjectCommon.js | 100 ++++++++++++++++++++++ www/manager6/panel/IPSet.js | 31 ++++++- 4 files changed, 181 insertions(+), 25 deletions(-) create mode 100644 www/manager6/grid/FirewallObjectCommon.js Summary over all repositories: 9 files changed, 663 insertions(+), 39 deletions(-) -- Generated by murpp 0.12.0