public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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




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