[PATCH pve-firewall v3 03/16] api: ipset: add option to update references on edit

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

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 <a.bied-charreton@proxmox.com>
---
 src/PVE/API2/Firewall/IPSet.pm | 60 +++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 82b9aaf..e734b8c 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -534,6 +534,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;
@@ -661,6 +662,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 => '',
@@ -681,6 +692,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}) {
@@ -690,19 +703,42 @@ sub register_create {
                         raise_param_exc({ name => "IPSet '$param->{rename}' does not exist" })
                             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}) {
+                            # prevent overwriting existing ipset
+                            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