[PATCH pve-firewall v4 08/27] firewall: aliases: add option to handle dangling references on rename

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

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 <a.bied-charreton@proxmox.com>
---
 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