[PATCH pve-firewall v6 15/28] fix #8176: firewall: groups: handle dangling references on delete

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

Deleting a security group referenced by rules leaves dangling
references. Nothing checks that a group rule names a group that exists.
The dangling reference is carried into the generated ruleset and what
happens then depends on the firewall implementation.

Add a 'dangling-references' option to the delete endpoint, to either
'disable' the referencing rules or 'drop' them along with the security
group. The default, 'keep', leaves them as they are.

If that option is passed, the delete runs as a task so the changes made
to the configs can be found in the task log afterwards.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=8176
Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/API2/Firewall/Rules.pm | 49 +++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index 785ad05..9931d0c 100644
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -487,6 +487,7 @@ package PVE::API2::Firewall::GroupRules;
 use strict;
 use warnings;
 use PVE::JSONSchema qw(get_standard_option);
+use PVE::Firewall::References qw(update_refs run_ref_update);
 
 use base qw(PVE::API2::Firewall::RulesBase);
 
@@ -539,27 +540,51 @@ __PACKAGE__->register_method({
         additionalProperties => 0,
         properties => {
             group => get_standard_option('pve-security-group-name'),
+            'dangling-references' => get_standard_option('pve-dangling-references-delete'),
         },
     },
-    returns => { type => 'null' },
+    returns => { type => 'string', optional => 1 },
     code => sub {
         my ($param) = @_;
 
-        __PACKAGE__->lock_config(
-            $param,
-            sub {
-                my ($param) = @_;
+        my $action = $param->{'dangling-references'} // 'keep';
 
-                my (undef, $cluster_conf, $rules) = __PACKAGE__->load_config($param);
+        my $code = sub {
+            __PACKAGE__->lock_config(
+                $param,
+                sub {
+                    my ($param) = @_;
 
-                die "Security group '$param->{group}' is not empty\n"
-                    if scalar(@$rules);
+                    my (undef, $cluster_conf, $rules) = __PACKAGE__->load_config($param);
 
-                __PACKAGE__->save_rules($param, $cluster_conf, undef);
-            },
-        );
+                    die "Security group '$param->{group}' is not empty\n"
+                        if scalar(@$rules);
+
+                    update_refs({
+                        conf => $cluster_conf,
+                        kind => 'groups',
+                        old => $param->{group},
+                        env => 'cluster',
+                        action => $action eq 'keep' ? undef : $action,
+                        subject => "security group '$param->{group}' deleted",
+                    });
+
+                    __PACKAGE__->save_rules($param, $cluster_conf, undef);
+                },
+            );
+        };
+
+        if (!defined($param->{'dangling-references'})) {
+            $code->();
+            return undef;
+        }
+
+        # fail on the request instead of inside the task.
+        my (undef, undef, $rules) = __PACKAGE__->load_config($param);
+
+        die "Security group '$param->{group}' is not empty\n" if scalar($rules->@*);
 
-        return undef;
+        return run_ref_update('fw-group-delete', $param->{group}, $code);
     },
 });
 
-- 
2.47.3