[PATCH pve-firewall 2/5] ipset: Add option to update references on rename/delete

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

Renaming or deleting an IPSet 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/ipset 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.

Renaming a cluster ipset referenced by 10.000 different config files in
a 3-node test cluster takes about 4.8 seconds on my machine. Doing the
same with an unreferenced ipset (i.e. config files checked but not
written back due to lack of changes) takes about 2 seconds.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/API2/Firewall/IPSet.pm | 108 ++++++++++++++++++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 43a51a7..9896fdd 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.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);
@@ -128,6 +129,92 @@ sub register_get_ipset {
     });
 }
 
+# Apply $rewrite to each rule in $conf referencing $ipset, modifying $conf in place.
+#
+# $rewrite ->($rule) is expected to return the new rule, or undef if the goal is to remove matches.
+#
+# 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 IPSets with the same name may be defined at the
+# cluster level and at the guest level, in which case the guest's IPSet shadows the cluster's.
+sub rewrite_ipset_refs_in_conf {
+    my ($conf, $ipset, $rule_env, $is_guest, $rewrite) = @_;
+    my $modified = 0;
+
+    my @patterns;
+    if ($rule_env eq 'cluster') {
+        @patterns = ("+dc/$ipset");
+        # Guest config ipsets may shadow cluster config ones
+        push @patterns, "+$ipset" if !($is_guest && $conf->{ipset}->{$ipset});
+    } else {
+        @patterns = ("+$ipset", "+guest/$ipset");
+    }
+
+    my $rule_matches = sub {
+        my ($rule) = @_;
+        grep { ($rule->{source} // '') eq $_ || ($rule->{dest} // '') eq $_ } @patterns;
+    };
+
+    ($conf->{rules}, my $changed) = filter_map($conf->{rules}, $rewrite, $rule_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 in the current environment ($rule_env) referencing $ipset, 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 IPSets defined
+# in the cluster config.
+sub rewrite_ipset_refs_in_env {
+    my ($conf, $ipset, $rule_env, $rewrite) = @_;
+    return foreach_conf_in_env(
+        $conf,
+        $rule_env,
+        sub {
+            my ($fw_conf, $effective_env, $is_guest) = @_;
+            return rewrite_ipset_refs_in_conf(
+                $fw_conf, $ipset, $effective_env, $is_guest, $rewrite,
+            );
+        },
+    );
+}
+
+sub delete_ipset_refs {
+    my ($conf, $ipset, $rule_env) = @_;
+    return rewrite_ipset_refs_in_env($conf, lc($ipset), $rule_env, sub { undef });
+}
+
+sub rename_ipset_refs {
+    my ($conf, $ipset, $new_name, $rule_env) = @_;
+    my $lc_ipset = lc($ipset);
+    return rewrite_ipset_refs_in_env(
+        $conf,
+        $lc_ipset,
+        $rule_env,
+        sub {
+            my ($rule) = @_;
+            for my $field (qw(source dest)) {
+                my $val = lc($rule->{$field} // '');
+                if ($val eq "+dc/$lc_ipset") { $rule->{$field} = "+dc/$new_name" }
+                elsif ($val eq "+$lc_ipset") { $rule->{$field} = "+$new_name" }
+                elsif ($val eq "+guest/$lc_ipset") { $rule->{$field} = "+guest/$new_name" }
+            }
+            return $rule;
+        },
+    );
+}
+
 sub register_delete_ipset {
     my ($class) = @_;
 
@@ -139,6 +226,12 @@ sub register_delete_ipset {
         optional => 1,
         description => 'Delete all members of the IPSet, if there are any.',
     };
+    $properties->{'delete-references'} = {
+        type => 'boolean',
+        optional => 1,
+        description => 'Delete dangling references after deleting the IPSet',
+        default => 0,
+    };
 
     $class->register_method({
         name => 'delete_ipset',
@@ -165,8 +258,10 @@ sub register_delete_ipset {
                     die "IPSet '$param->{name}' is not empty\n"
                         if scalar(@$ipset) && !$param->{force};
 
-                    $class->save_ipset($param, $fw_conf, undef);
+                    delete_ipset_refs($fw_conf, $param->{name}, $class->rule_env())
+                        if $param->{'delete-references'};
 
+                    $class->save_ipset($param, $fw_conf, undef);
                 },
             );
 
@@ -654,6 +749,13 @@ sub register_create {
         },
     );
 
+    $properties->{'update-references'} = {
+        type => 'boolean',
+        optional => 1,
+        description => 'Update dangling references when renaming an IPSet.',
+        default => 0,
+    };
+
     $class->register_method({
         name => 'create_ipset',
         path => '',
@@ -688,6 +790,10 @@ sub register_create {
                             if $fw_conf->{ipset}->{ $param->{name} }
                             && $param->{name} ne $param->{rename};
 
+                        PVE::API2::Firewall::IPSetBase::rename_ipset_refs(
+                            $fw_conf, $param->{rename}, $param->{name}, $class->rule_env(),
+                        ) if $param->{'update-references'};
+
                         my $data = delete $fw_conf->{ipset}->{ $param->{rename} };
                         $fw_conf->{ipset}->{ $param->{name} } = $data;
                         if (
-- 
2.47.3