Merge branch 'topic/awelzel/3112-log-suffix-left-over-log-rotation'

* topic/awelzel/3112-log-suffix-left-over-log-rotation:
  cluster/logger: Fix leftover-log-rotation in multi-logger setups
  cluster/logger: Fix global var reference

CHANGES

@@ -1,3 +1,19 @@
+6.1.0-dev.28 | 2023-06-13 17:33:31 +0200
+
+  * GH-3112: cluster/logger: Fix leftover-log-rotation in multi-logger setups (Arne Welzel, Corelight)
+
+    Populating log_metadata during zeek_init() is too late for the
+    leftover-log-rotation functionality, so do it at script parse time.
+
+    Also, prepend archiver_ to the log_metadata table and encoding function
+    due to being in the global namespace and to align with the
+    archiver_rotation_format_func. This hasn't been in a released
+    version yet, so fine to rename still.
+
+    Closes #3112
+
+  * cluster/logger: Fix global var reference (Arne Welzel, Corelight)
+
 6.1.0-dev.25 | 2023-06-12 15:27:20 -0700
 
   * Update broker submodule [nomail] (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-6.1.0-dev.25
+6.1.0-dev.28

scripts/base/frameworks/cluster/nodes/logger.zeek

@@ -22,16 +22,24 @@ redef Log::default_rotation_interval = 1 hrs;
 ## Alarm summary mail interval.
 redef Log::default_mail_alarms_interval = 24 hrs;
 
-## Generic log metadata rendered into the filename that zeek-archiver may interpret.
-## This is populated with a log_suffix entry within zeek_init() when multiple
-## logger nodes are defined in cluster-layout.zeek.
-global log_metadata: table[string] of string;
+## Generic log metadata rendered into filename that zeek-archiver may interpret.
+global archiver_log_metadata: table[string] of string &redef;
+
+# Populate archiver_log_metadata with a "log_suffix" entry when multiple
+# loggers are configured in Cluster::nodes. Need to evaluate at script
+# loading time as leftover-log-rotation functionality is invoking
+# archiver_rotation_format_func early on during InitPostScript().
+@if ( Cluster::get_node_count(Cluster::LOGGER) > 1 )
+redef archiver_log_metadata += {
+	["log_suffix"] = Cluster::node,
+};
+@endif
 
 ## Encode the given table as zeek-archiver understood metadata part.
-function encode_log_metadata(tbl: table[string] of string): string
+function archiver_encode_log_metadata(tbl: table[string] of string): string
 	{
 	local metadata_vec: vector of string;
-	for ( k, v in log_metadata )
+	for ( k, v in tbl )
 		{
 		if ( |v| == 0 )  # Assume concious decision to skip this entry.
 			next;
@@ -57,8 +65,8 @@ function archiver_rotation_format_func(ri: Log::RotationFmtInfo): Log::RotationP
 	local close_str = strftime(Log::default_rotation_date_format, ri$close);
 	local base = fmt("%s__%s__%s__", ri$path, open_str, close_str);
 
-	if ( |log_metadata| > 0 )
-		base = fmt("%s%s__", base, encode_log_metadata(log_metadata));
+	if ( |archiver_log_metadata| > 0 )
+		base = fmt("%s%s__", base, archiver_encode_log_metadata(archiver_log_metadata));
 
 	local rval = Log::RotationPath($file_basename=base);
 	return rval;
@@ -71,15 +79,6 @@ redef Log::default_rotation_dir = "log-queue";
 redef Log::rotation_format_func = archiver_rotation_format_func;
 
 redef LogAscii::enable_leftover_log_rotation = T;
-
-event zeek_init()
-	{
-	if ( "log_suffix" in log_metadata )
-		return;
-
-	if ( Cluster::get_node_count(Cluster::LOGGER) > 1 )
-		log_metadata["log_suffix"] = Cluster::node;
-	}
 @else
 
 ## Use the cluster's archive logging script.

testing/btest/Baseline/scripts.base.frameworks.cluster.leftover-log-rotation-multi-logger/out

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+./log-queue/conn__XXXX-XX-XX-XX-XX-XX__XXXX-XX-XX-XX-XX-XX__log_suffix=logger-2__.log
+./log-queue/dns__XXXX-XX-XX-XX-XX-XX__XXXX-XX-XX-XX-XX-XX__log_suffix=logger-2__.log
+leftover conn log
+leftover dns log

testing/btest/scripts/base/frameworks/cluster/leftover-log-rotation-multi-logger.zeek

@@ -0,0 +1,44 @@
+# @TEST-DOC: Ensure that left-over log rotation tags the logger name on as well.
+
+# @TEST-EXEC: echo ".log" >> .shadow.conn.log
+# @TEST-EXEC: echo "archiver_rotation_format_func" >> .shadow.conn.log
+# @TEST-EXEC: echo "leftover conn log" > conn.log
+
+# @TEST-EXEC: echo ".log" >> .shadow.dns.log
+# @TEST-EXEC: echo "archiver_rotation_format_func" >> .shadow.dns.log
+# @TEST-EXEC: echo "leftover dns log" > dns.log
+
+# Start Zeek as cluster node logger-2.
+# @TEST-EXEC: CLUSTER_NODE=logger-2 zeek -b %INPUT > out
+
+# Ensure leftover files were removed.
+# @TEST-EXEC: ! test -f .shadow.conn.log
+# @TEST-EXEC: ! test -f conn.log
+# @TEST-EXEC: ! test -f .shadow.dns.log
+# @TEST-EXEC: ! test -f dns.log
+
+# Ensure the rotated files end-up in the default log-queue directory and have
+# the logger-2 name encoded into them.
+# @TEST-EXEC: ls ./log-queue/conn__*.log >>out
+# @TEST-EXEC: ls ./log-queue/dns__*.log >>out
+# @TEST-EXEC: cat ./log-queue/conn__*logger-2__.log ./log-queue/dns__*logger-2__.log >>out
+
+# @TEST-EXEC: TEST_DIFF_CANONIFIER='sed -r "s/[0-9]{2}/XX/g"'  btest-diff out
+
+@TEST-START-FILE cluster-layout.zeek
+redef Cluster::nodes = {
+        ["logger-1"] = [$node_type=Cluster::LOGGER,  $ip=127.0.0.1, $p=1234/tcp],
+        ["logger-2"] = [$node_type=Cluster::LOGGER,  $ip=127.0.0.1, $p=1235/tcp],
+};
+@TEST-END-FILE
+
+# Switch settings into a supervisor/non-zeekctl setup
+redef Log::default_rotation_dir = "log-queue";
+redef Log::rotation_format_func = archiver_rotation_format_func;
+redef LogAscii::enable_leftover_log_rotation = T;
+redef Log::default_rotation_postprocessor_cmd = "";
+
+event zeek_init()
+	{
+	terminate();
+	}