From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-firewall 3/5] aliases: Add option to update references on rename/delete
Date: Tue, 7 Apr 2026 09:36:56 +0200 [thread overview]
Message-ID: <20260407073658.90818-4-a.bied-charreton@proxmox.com> (raw)
In-Reply-To: <20260407073658.90818-1-a.bied-charreton@proxmox.com>
Renaming or deleting an alias that is referenced by rules or security
groups would leave dangling references, creating broken configurations.
Add option to the POST and DELETE endpoints for /firewall/aliases to
update or delete those references from cluster, hosts, and guests
firewall configs.
If these endpoints are hit from the cluster environment (/cluster/),
all firewall config files in the cluster (cluster + all nodes + all
guests) have to be checked and possibly updated.
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
src/PVE/API2/Firewall/Aliases.pm | 115 +++++++++++++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm
index eaafe68..2a06822 100644
--- a/src/PVE/API2/Firewall/Aliases.pm
+++ b/src/PVE/API2/Firewall/Aliases.pm
@@ -5,6 +5,7 @@ use warnings;
use PVE::Exception qw(raise raise_param_exc);
use PVE::JSONSchema qw(get_standard_option);
+use PVE::API2::Firewall::Helpers qw(filter_map foreach_conf_in_env);
use PVE::Firewall;
use base qw(PVE::RESTHandler);
@@ -217,6 +218,12 @@ sub register_update_alias {
$properties->{cidr} = $api_properties->{cidr};
$properties->{comment} = $api_properties->{comment};
$properties->{digest} = get_standard_option('pve-config-digest');
+ $properties->{'update-references'} = {
+ type => 'boolean',
+ optional => 1,
+ description => 'Update dangling references when renaming the alias.',
+ default => 0,
+ };
$class->register_method({
name => 'update_alias',
@@ -261,6 +268,9 @@ sub register_update_alias {
if ($rename && ($name ne $rename)) {
raise_param_exc({ name => "alias '$param->{rename}' already exists" })
if defined($aliases->{$rename});
+ rename_alias_refs(
+ $fw_conf, $name, $param->{rename}, $class->rule_env(),
+ ) if $param->{'update-references'};
$aliases->{$name}->{name} = $param->{rename};
$aliases->{$rename} = $aliases->{$name};
delete $aliases->{$name};
@@ -282,6 +292,12 @@ sub register_delete_alias {
$properties->{name} = $api_properties->{name};
$properties->{digest} = get_standard_option('pve-config-digest');
+ $properties->{'delete-references'} = {
+ type => 'boolean',
+ optional => 1,
+ description => 'Delete dangling references after deleting the alias.',
+ default => 0,
+ };
$class->register_method({
name => 'remove_alias',
@@ -310,6 +326,10 @@ sub register_delete_alias {
PVE::Tools::assert_if_modified($digest, $param->{digest});
my $name = lc($param->{name});
+
+ delete_alias_refs($fw_conf, $param->{name}, $class->rule_env())
+ if $param->{'delete-references'};
+
delete $aliases->{$name};
$class->save_aliases($param, $fw_conf, $aliases);
@@ -321,6 +341,101 @@ sub register_delete_alias {
});
}
+# Apply $rewrite to each rule or IPSet entry in $conf referencing $alias, modifying $conf in place.
+#
+# $rewrite->($obj) is expected to return the new rule/entry, or undef to remove it.
+#
+# Returns a boolean indicating whether $conf was modified.
+#
+# If $is_guest is set to 1, references will be solved assuming $conf is a guest FW config.
+# This is important because 2 different aliases with the same name may be defined at the
+# cluster level and at the guest level, in which case the guest's alias shadows the cluster's.
+sub rewrite_alias_refs_in_conf {
+ my ($conf, $alias, $rule_env, $is_guest, $rewrite) = @_;
+ my $lc_alias = lc($alias);
+ my $modified = 0;
+
+ my @patterns;
+ if ($rule_env eq 'cluster') {
+ @patterns = ("dc/$lc_alias");
+ push @patterns, $lc_alias if !($is_guest && $conf->{aliases}->{$lc_alias});
+ } else {
+ @patterns = ($lc_alias, "guest/$lc_alias");
+ }
+
+ my $rule_matches = sub {
+ my ($rule) = @_;
+ grep { lc($rule->{source} // '') eq $_ || lc($rule->{dest} // '') eq $_ } @patterns;
+ };
+ my $entry_matches = sub {
+ grep { lc($_[0]->{cidr} // '') eq $_ } @patterns;
+ };
+
+ ($conf->{rules}, my $changed) = filter_map($conf->{rules}, $rewrite, $rule_matches);
+ $modified ||= $changed;
+
+ my $ipsets = $conf->{ipset} // {};
+ for my $name (keys %$ipsets) {
+ ($ipsets->{$name}, $changed) = filter_map($ipsets->{$name}, $rewrite, $entry_matches);
+ $modified ||= $changed;
+ }
+
+ if ($rule_env eq 'cluster') {
+ my $groups = $conf->{groups} // {};
+ for my $group (keys %$groups) {
+ ($groups->{$group}, $changed) = filter_map($groups->{$group}, $rewrite, $rule_matches);
+ $modified ||= $changed;
+ }
+ }
+
+ return $modified;
+}
+
+# Apply $rewrite to each rule or IPSet entry in the current environment ($rule_env) referencing
+# $alias, modifying $conf in place. The caller is responsible for locking and saving $conf.
+#
+# If $rule_env is 'cluster', this function will also map over all guest and node firewall configs
+# in the cluster, locking and saving them in the process, since those may reference aliases defined
+# in the cluster config.
+sub rewrite_alias_refs_in_env {
+ my ($conf, $alias, $rule_env, $rewrite) = @_;
+ return foreach_conf_in_env(
+ $conf,
+ $rule_env,
+ sub {
+ my ($fw_conf, $effective_env, $is_guest) = @_;
+ return rewrite_alias_refs_in_conf(
+ $fw_conf, $alias, $effective_env, $is_guest, $rewrite,
+ );
+ },
+ );
+}
+
+sub delete_alias_refs {
+ my ($conf, $alias, $rule_env) = @_;
+ return rewrite_alias_refs_in_env($conf, $alias, $rule_env, sub { undef });
+}
+
+sub rename_alias_refs {
+ my ($conf, $alias, $new_name, $rule_env) = @_;
+ my $lc_alias = lc($alias);
+ return rewrite_alias_refs_in_env(
+ $conf,
+ $lc_alias,
+ $rule_env,
+ sub {
+ my ($obj) = @_;
+ for my $field (qw(source dest cidr)) {
+ my $val = lc($obj->{$field} // '');
+ if ($val eq "dc/$lc_alias") { $obj->{$field} = "dc/$new_name" }
+ elsif ($val eq $lc_alias) { $obj->{$field} = $new_name }
+ elsif ($val eq "guest/$lc_alias") { $obj->{$field} = "guest/$new_name" }
+ }
+ return $obj;
+ },
+ );
+}
+
sub register_handlers {
my ($class) = @_;
--
2.47.3
next prev parent reply other threads:[~2026-04-07 7:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-07 7:36 [PATCH firewall/manager 0/5] Allow updating references to firewall objects when editing them Arthur Bied-Charreton
2026-04-07 7:36 ` [PATCH pve-firewall 1/5] Add helpers for updating alias and ipset references Arthur Bied-Charreton
2026-04-07 7:36 ` [PATCH pve-firewall 2/5] ipset: Add option to update references on rename/delete Arthur Bied-Charreton
2026-04-07 7:36 ` Arthur Bied-Charreton [this message]
2026-04-07 7:36 ` [PATCH pve-manager 4/5] " Arthur Bied-Charreton
2026-04-07 7:36 ` [PATCH pve-manager 5/5] aliases: " Arthur Bied-Charreton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260407073658.90818-4-a.bied-charreton@proxmox.com \
--to=a.bied-charreton@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.