[PATCH pve-firewall v5 03/27] firewall: references: log the references an object update touched

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

Make update_refs() log each reference as it is rewritten, so a run that
dies halfway through the cluster still shows what it got to:

    firewall: ipset 'set' renamed to 'set2':
      cluster: rules: rule 6: '+set' renamed to '+set2'
      cluster: group 'fixgrp': rule 1: '+set' renamed to '+set2'
    firewall: 2 references updated

The footer only counts what was actually updated, so it is logged even
if the walk did not run to the end. Errors are still raised.

The lines go to stdout in a worker, and to the syslog otherwise. Also
take an optional log sink coderef to allow a caller to collect logs
instead.

Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
 src/PVE/Firewall/References.pm | 93 +++++++++++++++++++++++++++++++---
 1 file changed, 87 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Firewall/References.pm b/src/PVE/Firewall/References.pm
index 8998380..fb25bc2 100644
--- a/src/PVE/Firewall/References.pm
+++ b/src/PVE/Firewall/References.pm
@@ -6,6 +6,8 @@ use warnings;
 use PVE::Cluster;
 use PVE::Firewall;
 use PVE::Network::SDN::Vnets;
+use PVE::RESTEnvironment;
+use PVE::SafeSyslog qw(syslog);
 
 use base 'Exporter';
 our @EXPORT_OK = qw(
@@ -71,7 +73,11 @@ sub rewrite_refs_in_env {
         my ($fw_conf, $env, $is_guest, $scope) = @_;
 
         my $conf_changes = rewrite_refs_in_conf($fw_conf, $op, $env, $is_guest);
-        push $changes->@*, map { { $_->%*, scope => $scope } } $conf_changes->@*;
+        for my $change ($conf_changes->@*) {
+            my $scoped = { $change->%*, scope => $scope };
+            push $changes->@*, $scoped;
+            $op->{on_change}->($scoped) if $op->{on_change};
+        }
 
         return scalar($conf_changes->@*);
     };
@@ -317,9 +323,16 @@ sub rewrite_refs_in_conf {
     return $ctx->{changes};
 }
 
+my $ref_change_verbs = {
+    rename => 'updated',
+    disable => 'disabled',
+    drop => 'dropped',
+};
+
 =head3 update_refs($opts)
 
-Rename, disable or drop all references to a firewall object across the environment.
+Rename, disable or drop all references to a firewall object across the environment, logging each
+one as it is rewritten.
 
 References are matched in rules, security groups and IPSet members. Matching is case-insensitive
 and renames are written back lowercased.
@@ -333,6 +346,10 @@ C<$opts> holds:
     env      # 'cluster', 'sdn', or 'vm'/'ct' for a guest-defined object
     action   # 'rename', 'disable' or 'drop', or undef to leave the references alone
     save     # persists C<conf>, required for a cluster-wide rename
+    subject  # what the log should call the operation
+    log      # where the logs go, task log if in worker or syslog by default
+
+The caller is responsible for locking and saving C<conf> itself.
 
 IPSet members have no disabled state, so C<disable> removes them as well.
 
@@ -342,7 +359,8 @@ C<conf> itself have been rewritten and before any downstream config is: downstre
 rewritten to reference the object under its new name, and a concurrent compilation parsing the
 cluster config would fail to resolve that name until it has been written.
 
-The caller is responsible for locking and saving C<conf> itself.
+If the logs cannot be written as we go (SDN apply is an example, since the conf update is done
+before forking a worker and logs would go to syslog), pass C<log> with custom collection logic.
 
 Return the number of references changed.
 
@@ -351,9 +369,15 @@ Return the number of references changed.
 sub update_refs {
     my ($opts) = @_;
 
-    my ($conf, $kind, $old, $new, $env, $action) = $opts->@{qw(conf kind old new env action)};
+    my ($conf, $kind, $old, $new, $env, $action, $subject) =
+        $opts->@{qw(conf kind old new env action subject)};
 
-    return 0 if !defined($action);
+    my $log = $opts->{log} // \&log_ref_line;
+
+    if (!defined($action)) {
+        $log->("firewall: $subject: references left as they are");
+        return 0;
+    }
 
     die "invalid action '$action'" if $action !~ m/^(rename|disable|drop)$/;
     die "\$new must be set if \$action is 'rename'" if $action eq 'rename' && !defined($new);
@@ -365,6 +389,10 @@ sub update_refs {
     die "cannot rename more than one object at once\n"
         if $action eq 'rename' && scalar($lc_old->@*) != 1;
 
+    $log->("firewall: $subject:");
+
+    my $count = 0;
+
     my $op = {
         kind => $kind,
         old => $lc_old,
@@ -372,9 +400,62 @@ sub update_refs {
         action => $action,
         cluster_conf => $conf,
         save => $opts->{save},
+        on_change => sub {
+            $count++;
+            $log->('  ' . format_ref_change($_[0]));
+        },
     };
 
-    return scalar(rewrite_refs_in_env($conf, $env, $op)->@*);
+    eval { rewrite_refs_in_env($conf, $env, $op) };
+    my $err = $@;
+
+    # if the walk died before incrementing $count there is nothing to log
+    if ($count) {
+        my $noun = $count == 1 ? 'reference' : 'references';
+        $log->("firewall: $count $noun $ref_change_verbs->{$action}");
+    } elsif (!$err) {
+        $log->("firewall: no references found");
+    }
+
+    die $err if $err;
+
+    return $count;
+}
+
+=head3 format_ref_change($change)
+
+Format one recorded config change as a single line, e.g.
+
+    "cluster: rules: rule 6: '+ipset' renamed to '+ipset1'"
+
+=cut
+
+sub format_ref_change {
+    my ($change) = @_;
+
+    my $section = $change->{section};
+    my $where = $section eq 'rules' ? 'rules' : "$section '$change->{name}'";
+    my $item = $section eq 'ipset' ? 'member' : 'rule';
+
+    my $what = "$change->{scope}: $where: $item $change->{pos}: '$change->{ref}'";
+
+    return "$what renamed to '$change->{new}'" if $change->{action} eq 'rename';
+    return "$what dropped" if $change->{action} eq 'drop';
+
+    # an IPSet member has no disabled state, so disabling it removed it
+    return "$what " . ($section eq 'ipset' ? 'removed' : 'disabled');
+}
+
+sub log_ref_line {
+    my ($line) = @_;
+
+    if (PVE::RESTEnvironment::is_worker()) {
+        print("$line\n");
+    } else {
+        syslog('info', '%s', $line);
+    }
+
+    return;
 }
 
 1;
-- 
2.47.3