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 9D0931FF0C2 for ; Thu, 22 Oct 2026 14:48:14 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 708272186E; Thu, 22 Oct 2026 14:47:24 +0200 (CEST) From: Arthur Bied-Charreton To: pve-devel@lists.proxmox.com Subject: [PATCH pve-firewall v4 08/27] firewall: aliases: add option to handle dangling references on rename Date: Thu, 22 Oct 2026 14:46:49 +0200 Message-ID: <20261022124708.429069-9-a.bied-charreton@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261022124708.429069-1-a.bied-charreton@proxmox.com> References: <20261022124708.429069-1-a.bied-charreton@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792673237086 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.891 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) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: X2PHHYW2AAX4RNK22ZZ4ZB4F2RGTLXKM X-Message-ID-Hash: X2PHHYW2AAX4RNK22ZZ4ZB4F2RGTLXKM X-MailFrom: a.bied-charreton@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 alias still referenced by rules or ipset members leaves dangling references. The firewall then fails to parse those rules during compilation and drops them. A rename can therefore effectively disable a whole set of rules. Add a 'dangling-references' option to the rename path offering to 'keep' the references around, 'disable' the referencing rules, 'drop' them, or to 'rename' the references to the new name. For cluster aliases, this also covers references in downstream configs (host, guest and vnet). A rename that fails partway leaves both the old and the new alias in the cluster config. Retrying it chokes on the check against overwriting an existing alias. 'force-rename' bypasses that check, so an interrupted rename can be finished. Avoiding the half-updated state altogether would need either a transaction across every config in the cluster or a rollback mechanism, neither of which is really an option here. Signed-off-by: Arthur Bied-Charreton --- src/PVE/API2/Firewall/Aliases.pm | 60 ++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm index 4f6960d..9e03de0 100644 --- a/src/PVE/API2/Firewall/Aliases.pm +++ b/src/PVE/API2/Firewall/Aliases.pm @@ -2,6 +2,8 @@ package PVE::API2::Firewall::AliasesBase; use strict; use warnings; + +use PVE::Firewall::References qw(update_refs); use PVE::Exception qw(raise raise_param_exc); use PVE::JSONSchema qw(get_standard_option); @@ -219,6 +221,19 @@ sub register_update_alias { $properties->{comment} = $api_properties->{comment}; $properties->{digest} = get_standard_option('pve-config-digest'); + $properties->{'dangling-references'} = { + type => 'string', + enum => ['keep', 'disable', 'drop', 'rename', 'force-rename'], + optional => 1, + description => + "Handle references that renaming the alias would leave dangling. Use 'rename' to " + . "point them at the new name, 'disable' to disable the referencing rules, or " + . "'drop' to remove them entirely. IPSet members have no disabled state and are " + . "removed in both cases. 'force-rename' also renames, but skips the check for an " + . "existing alias with the target name, to finish a rename interrupted partway.", + default => 'keep', + }; + $class->register_method({ name => 'update_alias', path => '{name}', @@ -239,6 +254,17 @@ sub register_update_alias { sub { my ($param) = @_; + my $dangling_refs = $param->{'dangling-references'} // 'keep'; + + # 'force-rename' only differs in skipping the check above, the references are + # renamed either way + my $ref_action; + if ($dangling_refs eq 'force-rename') { + $ref_action = 'rename'; + } elsif ($dangling_refs ne 'keep') { + $ref_action = $dangling_refs; + } + my ($fw_conf, $aliases) = $class->load_config($param); my $list = &$aliases_to_list($aliases); @@ -261,9 +287,37 @@ sub register_update_alias { if ($rename && ($name ne $rename)) { raise_param_exc({ name => "alias '$param->{rename}' already exists" }) - if defined($aliases->{$rename}); - $aliases->{$name}->{name} = $param->{rename}; - $aliases->{$rename} = $aliases->{$name}; + if defined($aliases->{$rename}) + && $dangling_refs ne 'force-rename'; + + $aliases->{$rename} = + { $aliases->{$name}->%*, name => $param->{rename} }; + + my $saved = 0; + my $save = sub { + $class->save_aliases($param, $fw_conf, $aliases); + $saved = 1; + }; + + eval { + update_refs({ + conf => $fw_conf, + kind => 'aliases', + old => $name, + new => $param->{rename}, + env => $class->rule_env(), + action => $ref_action, + subject => "alias '$name' renamed to '$rename'", + save => $save, + }); + }; + if (my $err = $@) { + die "rename interrupted, the alias exists under both names; " + . "retry with 'force-rename' to finish: $err" + if $saved; + die "could not update references to alias '$name': $err"; + } + delete $aliases->{$name}; } -- 2.47.3