[PATCH pve-firewall v4 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 leaves an account of 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 tally only counts what landed, so it is logged whether or not the
walk ran to the end. Errors are raised unchanged once that is written,
as what they mean for the caller's API is the caller's to phrase.

The lines go to stdout in a worker, and to the syslog otherwise, since
PVE::Daemon sends a daemon's stdout to /dev/null. Callers that have to
rewrite before their task exists can pass a log sink and replay the
lines in the worker.

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