From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id C94DD1FF0A7 for ; Tue, 18 Aug 2026 15:35:22 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id BCC5221662; Tue, 18 Aug 2026 15:34:24 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH pve-firewall v2 2/9] api: ipset: add option to update references on edit Date: Tue, 18 Aug 2026 15:34:06 +0200 Message-ID: <20260818133413.450776-3-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260818133413.450776-1-a.bied-charreton@proxmox.com> References: <20260818133413.450776-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.726 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 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: MV7WLLZO3757JS76UY2D4OY5EBX4T4GA X-Message-ID-Hash: MV7WLLZO3757JS76UY2D4OY5EBX4T4GA 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 an ipset still referenced by rules leaves dangling references. The firewall then fails to parse those rules during compilation and drops them. The errors, while logged to the journal, are not visible from the GUI - a rename can therefore effectively disable a whole set of rules. Add an 'update-references' option to the rename path to rewrite them to the new name. For cluster ipsets, this also covers references in downstream configs (host, guest and vnet). The new ipset is persisted before its references are rewritten, so a concurrent firewall compilation never observes a dangling reference. If the cluster-wide rewrite is interrupted, it can be retried by passing 'update-references=force'. Signed-off-by: Arthur Bied-Charreton --- src/PVE/API2/Firewall/IPSet.pm | 59 +++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm index ca7228b..0d802aa 100644 --- a/src/PVE/API2/Firewall/IPSet.pm +++ b/src/PVE/API2/Firewall/IPSet.pm @@ -528,6 +528,7 @@ package PVE::API2::Firewall::BaseIPSetList; use strict; use warnings; +use PVE::Firewall::Helpers qw(update_refs get_object_spec); use PVE::JSONSchema qw(get_standard_option); use PVE::Exception qw(raise_param_exc); use PVE::Firewall; @@ -655,6 +656,16 @@ sub register_create { }, ); + $properties->{'update-references'} = { + type => 'string', + enum => ['no', 'yes', 'force'], + optional => 1, + description => + "Update all references to the IPSet when renaming it. Use 'force' to also " + . "overwrite an existing target IPSet, e.g. to resume an interrupted rename.", + default => 'no', + }; + $class->register_method({ name => 'create_ipset', path => '', @@ -675,6 +686,8 @@ sub register_create { sub { my ($param) = @_; + my $update_references = $param->{'update-references'} // 'no'; + my ($cluster_conf, $fw_conf) = $class->load_config($param); if ($param->{rename}) { @@ -685,18 +698,42 @@ sub register_create { if !$fw_conf->{ipset}->{ $param->{rename} }; # prevent overwriting existing ipset - raise_param_exc({ name => "IPSet '$param->{name}' does already exist" }) - if $fw_conf->{ipset}->{ $param->{name} } - && $param->{name} ne $param->{rename}; - - my $data = delete $fw_conf->{ipset}->{ $param->{rename} }; - $fw_conf->{ipset}->{ $param->{name} } = $data; - if ( - my $comment = - delete $fw_conf->{ipset_comments}->{ $param->{rename} } - ) { - $fw_conf->{ipset_comments}->{ $param->{name} } = $comment; + + if ($param->{name} ne $param->{rename}) { + raise_param_exc({ + name => "IPSet '$param->{name}' does already exist" }) + if $fw_conf->{ipset}->{ $param->{name} } + && ($update_references // '') ne 'force'; + + $fw_conf->{ipset}->{ $param->{name} } = + $fw_conf->{ipset}->{ $param->{rename} }; + + if ($update_references ne 'no') { + my $env = $class->rule_env(); + my $spec = get_object_spec('ipset'); + my $old = $param->{rename}; + my $new = $param->{name}; + + # persist the new ipset before rewriting references so a concurrent + # compilation never sees a reference to a not-yet-saved ipset. + $class->save_config($param, $fw_conf) if $env eq 'cluster'; + + eval { update_refs($fw_conf, $spec, $old, $new, $env) }; + die "rename interrupted, references may be partially updated; " + . "retry with 'force' to finish: $@" + if $@; + } + + delete $fw_conf->{ipset}->{ $param->{rename} }; + + if ( + my $comment = + delete $fw_conf->{ipset_comments}->{ $param->{rename} } + ) { + $fw_conf->{ipset_comments}->{ $param->{name} } = $comment; + } } + $fw_conf->{ipset_comments}->{ $param->{name} } = $param->{comment} if defined($param->{comment}); } else { -- 2.47.3