From a98ec6b08b8c79ea97e3e7a3a244fb2c8cc589e7 Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Mon, 24 Jun 2024 22:06:58 -0700 Subject: [PATCH 1/2] Provide a script-layer equivalent to Supervisor::__init_cluster(). If the script layer is able to access the current node's config via Supervisor::node(), it can handle populating Cluster::nodes. That code is much more straightforward than an equivalent in-core implementation (especially with the upcoming change to the cluster table's implementation). This introduces base/frameworks/cluster/supervisor.zeek and Cluster::Supervisor::__init_cluster_nodes() for that purpose. The @load of the Supervisor API in cluster/main.zeek isn't technically necessary since we already load it explicitly even in init-bare.zeek, but being explicit seems better. --- .../base/frameworks/cluster/supervisor.zeek | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 scripts/base/frameworks/cluster/supervisor.zeek diff --git a/scripts/base/frameworks/cluster/supervisor.zeek b/scripts/base/frameworks/cluster/supervisor.zeek new file mode 100644 index 0000000000..4e6ec51fff --- /dev/null +++ b/scripts/base/frameworks/cluster/supervisor.zeek @@ -0,0 +1,57 @@ +##! Cluster-related functionality specific to running under the Supervisor +##! framework. + +@load base/frameworks/supervisor/api + +module Cluster::Supervisor; + +export { + ## Populates the current node's :zeek:id:`Cluster::nodes` table from the + ## supervisor's node configuration in :zeek:id:`Supervisor::NodeConfig`. + ## + ## Returns: true if initialization completed, false otherwise. + global __init_cluster_nodes: function(): bool; +} + +function __init_cluster_nodes(): bool + { + local config = Supervisor::node(); + + if ( |config$cluster| == 0 ) + return F; + + local rolemap: table[Supervisor::ClusterRole] of Cluster::NodeType = { + [Supervisor::LOGGER] = Cluster::LOGGER, + [Supervisor::MANAGER] = Cluster::MANAGER, + [Supervisor::PROXY] = Cluster::PROXY, + [Supervisor::WORKER] = Cluster::WORKER, + }; + + local manager_name = ""; + local cnode: Cluster::Node; + local typ: Cluster::NodeType = Cluster::NONE; + + for ( node_name, endp in config$cluster ) + { + if ( endp$role == Supervisor::MANAGER ) + manager_name = node_name; + } + + for ( node_name, endp in config$cluster ) + { + if ( endp$role in rolemap ) + typ = rolemap[endp$role]; + + cnode = [$node_type=typ, $ip=endp$host, $p=endp$p]; +@pragma push ignore-deprecations + if ( endp?$interface ) + cnode$interface = endp$interface; +@pragma pop ignore-deprecations + if ( |manager_name| > 0 && cnode$node_type != Cluster::MANAGER ) + cnode$manager = manager_name; + + Cluster::nodes[node_name] = cnode; + } + + return T; + } From 737b1a20138e3720b5b5e33eef69377e35ff46db Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Mon, 24 Jun 2024 22:19:29 -0700 Subject: [PATCH 2/2] Remove the Supervisor's internal ClusterEndpoint struct. This eliminates one place in which we currently need to mirror changes to the script-land Cluster::Node record. Instead of keeping an exact in-core equivalent, the Supervisor now treats the data structure as opaque, and stores the whole cluster table as a JSON string. We may replace the script-layer Supervisor::ClusterEndpoint in the future, using Cluster::Node directly. But that's a more invasive change that will affect how people invoke Supervisor::create() and similars. Relying on JSON for serialization has the side-effect of removing the Supervisor's earlier quirk of using 0/tcp, not 0/unknown, to indicate unused ports in the Supervisor::ClusterEndpoint record. --- NEWS | 4 + scripts/base/frameworks/cluster/__load__.zeek | 21 ++- .../frameworks/management/agent/main.zeek | 7 +- src/supervisor/Supervisor.cc | 150 +++--------------- src/supervisor/Supervisor.h | 40 +---- src/supervisor/supervisor.bif | 9 -- .../coverage.init-default/missing_loads | 1 + 7 files changed, 51 insertions(+), 181 deletions(-) diff --git a/NEWS b/NEWS index 5531f3a378..d757ddff05 100644 --- a/NEWS +++ b/NEWS @@ -151,6 +151,10 @@ Changed Functionality it aligns with the same requirement for traditional analyzers and enables customizing file handles for protocol-specific semantics. +- The Supervisor's API now returns NodeConfig records with a cluster table whose + ClusterEndpoints have a port value of 0/unknown, rather than 0/tcp, to + indicate that the node in question has no listening port. + Removed Functionality --------------------- diff --git a/scripts/base/frameworks/cluster/__load__.zeek b/scripts/base/frameworks/cluster/__load__.zeek index 47918e7d0d..a854302636 100644 --- a/scripts/base/frameworks/cluster/__load__.zeek +++ b/scripts/base/frameworks/cluster/__load__.zeek @@ -14,14 +14,21 @@ redef Broker::log_topic = Cluster::rr_log_topic; # Add a cluster prefix. @prefixes += cluster -# If this script isn't found anywhere, the cluster bombs out. -# Loading the cluster framework requires that a script by this name exists -# somewhere in the ZEEKPATH. The only thing in the file should be the -# cluster definition in the :zeek:id:`Cluster::nodes` variable. +@if ( Supervisor::is_supervised() ) +# When running a supervised cluster, populate Cluster::nodes from the node table +# the Supervisor provides to new Zeek nodes. The management framework configures +# the cluster this way. +@load ./supervisor +@if ( Cluster::Supervisor::__init_cluster_nodes() && Cluster::get_node_count(Cluster::LOGGER) > 0 ) +redef Cluster::manager_is_logger = F; +@endif +@endif -@if ( ! Supervisor::__init_cluster() ) -# When running a supervised cluster, Cluster::nodes is instead populated -# from the internal C++-layer directly via the above BIF. +@if ( |Cluster::nodes| == 0 ) +# Fall back to loading a cluster topology from cluster-layout.zeek. If Zeek +# cannot find this script in your ZEEKPATH, it will exit. The script should only +# contain the cluster definition in the :zeek:id:`Cluster::nodes` variable. +# The zeekctl tool manages this file for you. @load cluster-layout @endif diff --git a/scripts/policy/frameworks/management/agent/main.zeek b/scripts/policy/frameworks/management/agent/main.zeek index 6397313eab..7dbe963cf9 100644 --- a/scripts/policy/frameworks/management/agent/main.zeek +++ b/scripts/policy/frameworks/management/agent/main.zeek @@ -625,10 +625,9 @@ function get_nodes_request_finish(areq: Management::Request::Request) if ( node in g_nodes ) cns$state = g_nodes[node]$state; - # The supervisor's responses use 0/tcp (not 0/unknown) - # when indicating an unused port because its internal - # serialization always assumes TCP. - if ( sns$node$cluster[node]$p != 0/tcp ) + # The supervisor's responses use 0/unknown to indicate + # unused ports. (Prior to Zeek 7 this used to be 0/tcp.) + if ( sns$node$cluster[node]$p != 0/unknown ) cns$p = sns$node$cluster[node]$p; } else diff --git a/src/supervisor/Supervisor.cc b/src/supervisor/Supervisor.cc index a176f033e3..0585ec6467 100644 --- a/src/supervisor/Supervisor.cc +++ b/src/supervisor/Supervisor.cc @@ -18,6 +18,8 @@ #define RAPIDJSON_HAS_STDSTRING 1 #include +#include +#include extern "C" { #include "zeek/3rdparty/setsignal.h" @@ -1243,34 +1245,9 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromRecord(const RecordVal* node) rval.env[name] = v->GetVal()->AsStringVal()->ToStdString(); } - auto cluster_table_val = node->GetField("cluster")->AsTableVal(); - auto cluster_table = cluster_table_val->AsTable(); - - for ( const auto& cte : *cluster_table ) { - auto k = cte.GetHashKey(); - auto* v = cte.value; - - auto key = cluster_table_val->RecreateIndex(*k); - auto name = key->Idx(0)->AsStringVal()->ToStdString(); - auto rv = v->GetVal()->AsRecordVal(); - - Supervisor::ClusterEndpoint ep; - ep.role = static_cast(rv->GetFieldAs("role")); - ep.host = rv->GetFieldAs("host").AsString(); - ep.port = rv->GetFieldAs("p")->Port(); - - const auto& iface = rv->GetField("interface"); - - if ( iface ) - ep.interface = iface->AsStringVal()->ToStdString(); - - const auto& pcap_file = rv->GetField("pcap_file"); - - if ( pcap_file ) - ep.pcap_file = pcap_file->AsStringVal()->ToStdString(); - - rval.cluster.emplace(name, std::move(ep)); - } + auto cluster_table_val = node->GetField("cluster"); + auto re = std::make_unique("^_"); + rval.cluster = cluster_table_val->ToJSON(false, re.get())->ToStdString(); return rval; } @@ -1319,26 +1296,10 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromJSON(std::string_view json) { auto& cluster = j["cluster"]; - for ( auto it = cluster.MemberBegin(); it != cluster.MemberEnd(); ++it ) { - Supervisor::ClusterEndpoint ep; - - auto key = it->name.GetString(); - auto& val = it->value; - - auto& role_str = val["role"]; - ep.role = role_str_to_enum(role_str.GetString()); - - ep.host = val["host"].GetString(); - ep.port = val["p"]["port"].GetInt(); - - if ( auto it = val.FindMember("interface"); it != val.MemberEnd() ) - ep.interface = it->value.GetString(); - - if ( auto it = val.FindMember("pcap_file"); it != val.MemberEnd() ) - ep.pcap_file = it->value.GetString(); - - rval.cluster.emplace(key, std::move(ep)); - } + rapidjson::StringBuffer sb; + rapidjson::Writer writer(sb); + cluster.Accept(writer); + rval.cluster = sb.GetString(); return rval; } @@ -1349,7 +1310,7 @@ std::string Supervisor::NodeConfig::ToJSON() const { } RecordValPtr Supervisor::NodeConfig::ToRecord() const { - const auto& rt = BifType::Record::Supervisor::NodeConfig; + const auto& rt = id::find_type("Supervisor::NodeConfig"); auto rval = make_intrusive(rt); rval->AssignField("name", name); @@ -1401,27 +1362,18 @@ RecordValPtr Supervisor::NodeConfig::ToRecord() const { } auto tt = rt->GetFieldType("cluster"); - auto cluster_val = make_intrusive(std::move(tt)); - rval->AssignField("cluster", cluster_val); - - for ( const auto& e : cluster ) { - auto& name = e.first; - auto& ep = e.second; - auto key = make_intrusive(name); - const auto& ept = BifType::Record::Supervisor::ClusterEndpoint; - auto val = make_intrusive(ept); - - val->AssignField("role", BifType::Enum::Supervisor::ClusterRole->GetEnumVal(ep.role)); - val->AssignField("host", make_intrusive(ep.host)); - val->AssignField("p", val_mgr->Port(ep.port, TRANSPORT_TCP)); - - if ( ep.interface ) - val->AssignField("interface", *ep.interface); - - if ( ep.pcap_file ) - val->AssignField("pcap_file", *ep.pcap_file); - - cluster_val->Assign(std::move(key), std::move(val)); + auto json_res = detail::ValFromJSON(cluster, tt, Func::nil); + if ( auto val = std::get_if(&json_res) ) { + rval->AssignField("cluster", *val); + } + else { + // This should never happen: the JSON data comes from a table[string] of + // ClusterEndpoint and should therefore allow instantiation. Exiting + // here can be hard to debug. Other JSON code (see FromJSON()) fails + // silently when the JSON is misformatted. We just warn: + fprintf(stderr, "Could not parse %s's cluster table from '%s': %s\n", name.c_str(), cluster.c_str(), + std::get(json_res).c_str()); + rval->AssignField("cluster", make_intrusive(std::move(tt))); } return rval; @@ -1439,62 +1391,6 @@ RecordValPtr SupervisorNode::ToRecord() const { return rval; } -static ValPtr supervisor_role_to_cluster_node_type(BifEnum::Supervisor::ClusterRole role) { - static auto node_type = id::find_type("Cluster::NodeType"); - - switch ( role ) { - case BifEnum::Supervisor::LOGGER: return node_type->GetEnumVal(node_type->Lookup("Cluster", "LOGGER")); - case BifEnum::Supervisor::MANAGER: return node_type->GetEnumVal(node_type->Lookup("Cluster", "MANAGER")); - case BifEnum::Supervisor::PROXY: return node_type->GetEnumVal(node_type->Lookup("Cluster", "PROXY")); - case BifEnum::Supervisor::WORKER: return node_type->GetEnumVal(node_type->Lookup("Cluster", "WORKER")); - default: return node_type->GetEnumVal(node_type->Lookup("Cluster", "NONE")); - } -} - -bool SupervisedNode::InitCluster() const { - if ( config.cluster.empty() ) - return false; - - const auto& cluster_node_type = id::find_type("Cluster::Node"); - const auto& cluster_nodes_id = id::find("Cluster::nodes"); - const auto& cluster_manager_is_logger_id = id::find("Cluster::manager_is_logger"); - auto cluster_nodes = cluster_nodes_id->GetVal()->AsTableVal(); - auto has_logger = false; - std::optional manager_name; - - for ( const auto& e : config.cluster ) { - if ( e.second.role == BifEnum::Supervisor::MANAGER ) - manager_name = e.first; - else if ( e.second.role == BifEnum::Supervisor::LOGGER ) - has_logger = true; - } - - for ( const auto& e : config.cluster ) { - const auto& node_name = e.first; - const auto& ep = e.second; - - auto key = make_intrusive(node_name); - auto val = make_intrusive(cluster_node_type); - - auto node_type = supervisor_role_to_cluster_node_type(ep.role); - val->AssignField("node_type", std::move(node_type)); - val->AssignField("ip", make_intrusive(ep.host)); - val->AssignField("p", val_mgr->Port(ep.port, TRANSPORT_TCP)); - - // Remove in v7.1: Interface removed from Cluster::Node. - if ( ep.interface ) - val->AssignField("interface", *ep.interface); - - if ( manager_name && ep.role != BifEnum::Supervisor::MANAGER ) - val->AssignField("manager", *manager_name); - - cluster_nodes->Assign(std::move(key), std::move(val)); - } - - cluster_manager_is_logger_id->SetVal(val_mgr->Bool(! has_logger)); - return true; -} - void SupervisedNode::Init(Options* options) const { const auto& node_name = config.name; @@ -1546,7 +1442,7 @@ void SupervisedNode::Init(Options* options) const { } } - if ( ! config.cluster.empty() ) { + if ( ! config.cluster.empty() && config.cluster != "{}" ) { if ( setenv("CLUSTER_NODE", node_name.data(), true) == -1 ) { fprintf(stderr, "node '%s' failed to setenv: %s\n", node_name.data(), strerror(errno)); exit(1); diff --git a/src/supervisor/Supervisor.h b/src/supervisor/Supervisor.h index aa511e1209..641618dfde 100644 --- a/src/supervisor/Supervisor.h +++ b/src/supervisor/Supervisor.h @@ -110,35 +110,6 @@ public: std::string zeek_exe_path; }; - /** - * Configuration options that influence how a Supervised Zeek node - * integrates into the normal Zeek Cluster Framework. - */ - struct ClusterEndpoint { - /** - * The node's role within the cluster. E.g. manager, logger, worker. - */ - BifEnum::Supervisor::ClusterRole role; - /** - * The TCP port number at which the cluster node listens for connections. - */ - int port; - /** - * The host/IP at which the cluster node is listening for connections. - */ - std::string host; - /** - * The interface name from which the node read/analyze packets. - * Typically used by worker nodes. - */ - std::optional interface; - /** - * The PCAP file name from which the node read/analyze packets. - * Typically used by worker nodes. - */ - std::optional pcap_file; - }; - /** * Configuration options that influence behavior of a Supervised Zeek node. */ @@ -233,15 +204,16 @@ public: */ std::vector addl_user_scripts; /** - * Environment variables and values to define in the node. + * Environment variables and values to define in the node. */ std::map env; /** - * The Cluster Layout definition. Each node in the Cluster Framework - * knows about the full, static cluster topology to which it belongs. - * Entries in the map use node names for keys. + * The cluster layout definition. Each node in the Cluster Framework + * knows the full, static cluster topology to which it belongs. The + * layout is encoded as the JSON map resulting from ToJSON() on the + * corresponding cluster table in the script layer's NodeConfig record. */ - std::map cluster; + std::string cluster; }; /** diff --git a/src/supervisor/supervisor.bif b/src/supervisor/supervisor.bif index b749c773d5..d8bd06bd96 100644 --- a/src/supervisor/supervisor.bif +++ b/src/supervisor/supervisor.bif @@ -14,7 +14,6 @@ enum ClusterRole %{ WORKER, %} -type Supervisor::ClusterEndpoint: record; type Supervisor::Status: record; type Supervisor::NodeConfig: record; type Supervisor::NodeStatus: record; @@ -66,14 +65,6 @@ function Supervisor::__restart%(node: string%): bool return zeek::val_mgr->Bool(rval); %} -function Supervisor::__init_cluster%(%): bool - %{ - if ( zeek::Supervisor::ThisNode() ) - return zeek::val_mgr->Bool(zeek::Supervisor::ThisNode()->InitCluster()); - - return zeek::val_mgr->Bool(false); - %} - function Supervisor::__is_supervised%(%): bool %{ return zeek::val_mgr->Bool(zeek::Supervisor::ThisNode().has_value()); diff --git a/testing/btest/Baseline/coverage.init-default/missing_loads b/testing/btest/Baseline/coverage.init-default/missing_loads index fe23c7a04a..e16624e1fb 100644 --- a/testing/btest/Baseline/coverage.init-default/missing_loads +++ b/testing/btest/Baseline/coverage.init-default/missing_loads @@ -5,6 +5,7 @@ -./frameworks/cluster/nodes/proxy.zeek -./frameworks/cluster/nodes/worker.zeek -./frameworks/cluster/setup-connections.zeek +-./frameworks/cluster/supervisor.zeek -./frameworks/intel/cluster.zeek -./frameworks/netcontrol/cluster.zeek -./frameworks/openflow/cluster.zeek