Merge remote-tracking branch 'origin/topic/awelzel/fix-no-zero-timestamp-metadata'

* origin/topic/awelzel/fix-no-zero-timestamp-metadata:
  btest: Add test for Cluster::hello zero-timestamp
  EventMgr/Enqueue: Add automatic timestamp metadata to local events, only
  cluster and broker: Propagate zero-timestamp as metadata, too.

CHANGES

@@ -1,3 +1,19 @@
+8.0.0-dev.209 | 2025-05-26 16:08:44 +0200
+
+  * btest: Add test for Cluster::hello zero-timestamp (Arne Welzel, Corelight)
+
+  * EventMgr/Enqueue: Add automatic timestamp metadata to local events, only (Arne Welzel, Corelight)
+
+    It seems less surprising if only local events receive automatic network
+    timestamp metadata. For remote events the automatic value will most
+    likely be misleading.
+
+  * cluster and broker: Propagate zero-timestamp as metadata, too. (Arne Welzel, Corelight)
+
+    This will be cleaned up later to just pass all contained metadata from
+    a cluster event to the queued event, but for now do this here, otherwise
+    we break some internal tests.
+
 8.0.0-dev.204 | 2025-05-23 12:13:41 -0700
 
   * Redis: bump version of hiredis required (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-8.0.0-dev.204
+8.0.0-dev.209

src/Event.cc

@@ -165,8 +165,13 @@ void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID
     detail::EventMetadataVectorPtr meta;
 
     double ts = double(deprecated_ts);
-    if ( BifConst::EventMetadata::add_network_timestamp ) {
-        if ( ts < 0.0 ) // default -1.0, modify to current network_time
+    if ( src == util::detail::SOURCE_LOCAL && BifConst::EventMetadata::add_network_timestamp ) {
+        // If this is a local event and EventMetadata::add_network_timestamp is
+        // enabled automatically set the network timestamp for this event to the
+        // current network time when it is < 0 (default is -1.0).
+        //
+        // See the other Enqueue() implementation for the local motivation.
+        if ( ts < 0.0 )
             ts = run_state::network_time;
 
         // In v8.1 when the deprecated_ts parameters is gone: Just use run_state::network_time directly here.
@@ -184,9 +189,15 @@ void EventMgr::Enqueue(const EventHandlerPtr& h, Args vl, util::detail::SourceID
 
 void EventMgr::Enqueue(detail::EventMetadataVectorPtr meta, const EventHandlerPtr& h, Args vl,
                        util::detail::SourceID src, analyzer::ID aid, Obj* obj) {
-    if ( BifConst::EventMetadata::add_network_timestamp ) {
+    if ( src == util::detail::SOURCE_LOCAL && BifConst::EventMetadata::add_network_timestamp ) {
         // If all events are supposed to have a network time attached, ensure
         // that the meta vector was passed *and* contains a network timestamp.
+        //
+        // This is only done for local events, however. For remote events (src == BROKER)
+        // that do not hold network timestamp metadata, it seems less surprising to keep
+        // it unset. If it is required that a remote node sends *their* network timestamp,
+        // defaulting to this node's network time seems more confusing and error prone
+        // than just leaving it unset and having the consumer deal with the situation.
         bool has_time = false;
 
         if ( ! meta ) {

src/broker/Manager.cc

@@ -1657,7 +1657,7 @@ void Manager::ProcessMessage(std::string_view topic, broker::zeek::Event& ev) {
 
     if ( vl.size() == args.size() ) {
         zeek::detail::EventMetadataVectorPtr meta;
-        if ( ts > 0.0 )
+        if ( ts >= 0.0 )
             meta = zeek::detail::MakeEventMetadataVector(ts);
 
         event_mgr.Enqueue(std::move(meta), handler, std::move(vl), util::detail::SOURCE_BROKER);

src/cluster/Backend.cc

@@ -24,7 +24,7 @@ using namespace zeek::cluster;
 
 bool detail::LocalEventHandlingStrategy::DoProcessEvent(std::string_view topic, detail::Event e) {
     zeek::detail::EventMetadataVectorPtr meta;
-    if ( auto ts = e.Timestamp(); ts > 0.0 )
+    if ( auto ts = e.Timestamp(); ts >= 0.0 )
         meta = zeek::detail::MakeEventMetadataVector(e.Timestamp());
 
     zeek::event_mgr.Enqueue(std::move(meta), e.Handler(), std::move(e.Args()), util::detail::SOURCE_BROKER);

testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..manager..stdout

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+Cluster::hello name=worker-1 is_remote_event=T metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=0.0]]
+Cluster::node_up name=worker-1 is_remote_event=F metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=XXXXXXXXXX.XXXXXX]]

testing/btest/Baseline/cluster.broker.cluster-hello-zero-network-timestamp/..worker-1..stdout

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+Cluster::hello name=manager is_remote_event=T metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=XXXXXXXXXX.XXXXXX]]
+Cluster::node_up name=manager is_remote_event=F metadata=[[id=EventMetadata::NETWORK_TIMESTAMP, val=0.0]]

testing/btest/cluster/broker/cluster-hello-zero-network-timestamp.zeek

@@ -0,0 +1,48 @@
+# @TEST-DOC: Ensure Cluster::hello sent by a worker with a zero network time is observed with a zero network timestamp by the manager.
+#
+# @TEST-PORT: BROKER_MANAGER_PORT
+# @TEST-PORT: BROKER_WORKER1_PORT
+#
+# @TEST-EXEC: cp $FILES/broker/cluster-layout.zeek .
+#
+# @TEST-EXEC: zeek -b --parse-only %INPUT
+#
+# @TEST-EXEC: btest-bg-run manager "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=manager zeek -b %INPUT"
+# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=worker-1 zeek -b %INPUT"
+#
+# @TEST-EXEC: btest-bg-wait 30
+# @TEST-EXEC: btest-diff ./manager/.stdout
+# @TEST-EXEC: btest-diff ./worker-1/.stdout
+
+redef allow_network_time_forward = F;
+redef EventMetadata::add_network_timestamp = T;
+
+event do_terminate()
+	{
+	terminate();
+	}
+
+event zeek_init()
+	{
+	# Set the manager's time to non-zero, the worker continues to be at 0.0.
+	if ( Cluster::local_node_type() == Cluster::MANAGER )
+		set_network_time(double_to_time(1748256346));
+	}
+
+event Cluster::hello(name: string, id: string)
+	{
+	print fmt("Cluster::hello name=%s is_remote_event=%s metadata=%s", name, is_remote_event(), EventMetadata::current_all());
+	}
+
+event Cluster::node_up(name: string, id: string)
+	{
+	print fmt("Cluster::node_up name=%s is_remote_event=%s metadata=%s", name, is_remote_event(), EventMetadata::current_all());
+
+	if ( Cluster::local_node_type() == Cluster::MANAGER )
+		Cluster::publish(Cluster::worker_topic, do_terminate);
+	}
+
+event Cluster::node_down(name: string, id: string)
+	{
+	terminate();
+	}