From 0ad32101773c5845ae7fc6014abd251c418c6eac Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Wed, 11 Dec 2024 15:34:07 +0100 Subject: [PATCH] Broker::publish: Warn on using Broker::publish() when inactive This is mostly for transitioning base scripts to Cluster::publish() and avoid silent surprises why certain things don't work when using ZeroMQ. --- src/broker/Manager.h | 2 +- src/broker/messaging.bif | 6 ++ .../..manager..stderr | 4 ++ .../..manager.out | 3 + .../..worker.out | 2 + .../btest/cluster/broker/publish-warning.zeek | 61 +++++++++++++++++++ 6 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 testing/btest/Baseline/cluster.broker.publish-warning/..manager..stderr create mode 100644 testing/btest/Baseline/cluster.broker.publish-warning/..manager.out create mode 100644 testing/btest/Baseline/cluster.broker.publish-warning/..worker.out create mode 100644 testing/btest/cluster/broker/publish-warning.zeek diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 51974685d9..5b02a0ba71 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -109,7 +109,7 @@ public: /** * Returns true if any Broker communication is currently active. */ - [[deprecated("Remove with v8.1 - unused")]] bool Active(); + bool Active(); /** * Advances time. Broker data store expiration is driven by this diff --git a/src/broker/messaging.bif b/src/broker/messaging.bif index 96008861de..4347616a97 100644 --- a/src/broker/messaging.bif +++ b/src/broker/messaging.bif @@ -57,8 +57,14 @@ static bool publish_event_args(ArgsSpan args, const zeek::String* topic, zeek::detail::Frame* frame) { zeek::Broker::Manager::ScriptScopeGuard ssg; + zeek::ScriptLocationScope scope{frame}; + auto rval = false; + if ( zeek::broker_mgr != zeek::cluster::backend && ! zeek::broker_mgr->Active() ) + zeek::reporter->Warning("Non-broker cluster backend configured and Broker manager inactive. " + "Did you mean to use Cluster::publish() instead of Broker::publish()?"); + if ( args[0]->GetType()->Tag() == zeek::TYPE_RECORD ) rval = zeek::broker_mgr->PublishEvent(topic->CheckString(), args[0]->AsRecordVal()); diff --git a/testing/btest/Baseline/cluster.broker.publish-warning/..manager..stderr b/testing/btest/Baseline/cluster.broker.publish-warning/..manager..stderr new file mode 100644 index 0000000000..9e7871896a --- /dev/null +++ b/testing/btest/Baseline/cluster.broker.publish-warning/..manager..stderr @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +warning in ../manager.zeek, line 8: Non-broker cluster backend configured and Broker manager inactive. Did you mean to use Cluster::publish() instead of Broker::publish()? +warning in ../manager.zeek, line 16: Non-broker cluster backend configured and Broker manager inactive. Did you mean to use Cluster::publish() instead of Broker::publish()? +received termination signal diff --git a/testing/btest/Baseline/cluster.broker.publish-warning/..manager.out b/testing/btest/Baseline/cluster.broker.publish-warning/..manager.out new file mode 100644 index 0000000000..7e67339b4d --- /dev/null +++ b/testing/btest/Baseline/cluster.broker.publish-warning/..manager.out @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +node_up, worker-1 +node_down, worker-1 diff --git a/testing/btest/Baseline/cluster.broker.publish-warning/..worker.out b/testing/btest/Baseline/cluster.broker.publish-warning/..worker.out new file mode 100644 index 0000000000..386f8ae30f --- /dev/null +++ b/testing/btest/Baseline/cluster.broker.publish-warning/..worker.out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +node_up, manager diff --git a/testing/btest/cluster/broker/publish-warning.zeek b/testing/btest/cluster/broker/publish-warning.zeek new file mode 100644 index 0000000000..38dff8e980 --- /dev/null +++ b/testing/btest/cluster/broker/publish-warning.zeek @@ -0,0 +1,61 @@ +# @TEST-DOC: When using ZeroMQ, Broker::publish() produces a warning. +# +# @TEST-REQUIRES: have-zeromq +# +# @TEST-GROUP: cluster-zeromq +# +# @TEST-PORT: XPUB_PORT +# @TEST-PORT: XSUB_PORT +# @TEST-PORT: LOG_PULL_PORT +# +# @TEST-EXEC: cp $FILES/zeromq/cluster-layout-simple.zeek cluster-layout.zeek +# @TEST-EXEC: cp $FILES/zeromq/test-bootstrap.zeek zeromq-test-bootstrap.zeek +# +# @TEST-EXEC: btest-bg-run manager "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=manager zeek -b ../manager.zeek >out" +# @TEST-EXEC: btest-bg-run worker "ZEEKPATH=$ZEEKPATH:.. && CLUSTER_NODE=worker-1 zeek -b ../worker.zeek >out" +# +# @TEST-EXEC: btest-bg-wait 30 +# @TEST-EXEC: btest-diff ./manager/out +# @TEST-EXEC: btest-diff ./manager/.stderr +# @TEST-EXEC: btest-diff ./worker/out + + +# @TEST-START-FILE common.zeek +@load ./zeromq-test-bootstrap + +global finish: event(name: string); +# @TEST-END-FILE + +# @TEST-START-FILE manager.zeek +@load ./common.zeek +# If a node comes up that isn't us, send it a finish event. +event Cluster::node_up(name: string, id: string) { + print "node_up", name; + Cluster::publish(Cluster::nodeid_topic(id), finish, Cluster::node); + + # Also via broker, but this produces a warning which we test for. + Broker::publish(Cluster::nodeid_topic(id), finish, Cluster::node); +} + +# If the worker vanishes, finish the test. +event Cluster::node_down(name: string, id: string) { + print "node_down", name; + + # Do another Broker::publish(), just for the kicks. + Broker::publish(Cluster::nodeid_topic(id), finish, Cluster::node); + + terminate(); +} +# @TEST-END-FILE + +# @TEST-START-FILE worker.zeek +@load ./common.zeek + +event Cluster::node_up(name: string, id: string) { + print "node_up", name; +} + +event finish(name: string) &is_used { + terminate(); +} +# @TEST-END-FILE