From 4b5584a85d28d3e449254d80db43a5781d4dcbc6 Mon Sep 17 00:00:00 2001
From: Christian Kreibich <christian@corelight.com>
Date: Mon, 31 Jan 2022 18:13:15 -0800
Subject: [PATCH] Move redefs of ClusterController::Request::Request to their
 places of use

The Request module does not need to know about additional state tucked onto it
by its users.
---
 .../policy/frameworks/cluster/agent/main.zeek | 13 ++++++--
 .../frameworks/cluster/controller/main.zeek   | 18 +++++++++--
 .../cluster/controller/request.zeek           | 32 +++----------------
 3 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/scripts/policy/frameworks/cluster/agent/main.zeek b/scripts/policy/frameworks/cluster/agent/main.zeek
index f545186304..6c782734c1 100644
--- a/scripts/policy/frameworks/cluster/agent/main.zeek
+++ b/scripts/policy/frameworks/cluster/agent/main.zeek
@@ -13,6 +13,15 @@
 
 module ClusterAgent::Runtime;
 
+# Request state specific to supervisor interactions
+type SupervisorState: record {
+	node: string;
+};
+
+redef record ClusterController::Request::Request += {
+	supervisor_state: SupervisorState &optional;
+};
+
 redef ClusterController::role = ClusterController::Types::AGENT;
 
 # The global configuration as passed to us by the controller
@@ -69,7 +78,7 @@ event SupervisorControl::destroy_response(reqid: string, result: bool)
 function supervisor_create(nc: Supervisor::NodeConfig)
 	{
 	local req = ClusterController::Request::create();
-	req$supervisor_state = ClusterController::Request::SupervisorState($node = nc$name);
+	req$supervisor_state = SupervisorState($node = nc$name);
 	event SupervisorControl::create_request(req$id, nc);
 	ClusterController::Log::info(fmt("issued supervisor create for %s, %s", nc$name, req$id));
 	}
@@ -77,7 +86,7 @@ function supervisor_create(nc: Supervisor::NodeConfig)
 function supervisor_destroy(node: string)
 	{
 	local req = ClusterController::Request::create();
-	req$supervisor_state = ClusterController::Request::SupervisorState($node = node);
+	req$supervisor_state = SupervisorState($node = node);
 	event SupervisorControl::destroy_request(req$id, node);
 	ClusterController::Log::info(fmt("issued supervisor destroy for %s, %s", node, req$id));
 	}
diff --git a/scripts/policy/frameworks/cluster/controller/main.zeek b/scripts/policy/frameworks/cluster/controller/main.zeek
index d96b49d58f..b0fe565553 100644
--- a/scripts/policy/frameworks/cluster/controller/main.zeek
+++ b/scripts/policy/frameworks/cluster/controller/main.zeek
@@ -15,6 +15,20 @@
 
 module ClusterController::Runtime;
 
+# Request state specific to the set_configuration request/response events
+type SetConfigurationState: record {
+	config: ClusterController::Types::Configuration;
+	requests: set[string] &default=set();
+};
+
+# Dummy state for testing events.
+type TestState: record { };
+
+redef record ClusterController::Request::Request += {
+	set_configuration_state: SetConfigurationState &optional;
+	test_state: TestState &optional;
+};
+
 redef ClusterController::role = ClusterController::Types::CONTROLLER;
 
 global check_instances_ready: function();
@@ -323,7 +337,7 @@ event ClusterController::API::set_configuration_request(reqid: string, config: C
 	local res: ClusterController::Types::Result;
 	local req = ClusterController::Request::create(reqid);
 
-	req$set_configuration_state = ClusterController::Request::SetConfigurationState($config = config);
+	req$set_configuration_state = SetConfigurationState($config = config);
 
 	# At the moment there can only be one pending request.
 	if ( g_config_reqid_pending != "" )
@@ -486,7 +500,7 @@ event ClusterController::API::test_timeout_request(reqid: string, with_state: bo
 		# This state times out and triggers a timeout response in the
 		# above request_expired event handler.
 		local req = ClusterController::Request::create(reqid);
-		req$test_state = ClusterController::Request::TestState();
+		req$test_state = TestState();
 		}
 	}
 
diff --git a/scripts/policy/frameworks/cluster/controller/request.zeek b/scripts/policy/frameworks/cluster/controller/request.zeek
index 987dc9338c..4fc648b511 100644
--- a/scripts/policy/frameworks/cluster/controller/request.zeek
+++ b/scripts/policy/frameworks/cluster/controller/request.zeek
@@ -18,37 +18,13 @@ export {
 		## received by the client), this specifies that original, "parent"
 		## request.
 		parent_id: string &optional;
-	};
 
-	# API-specific state. XXX we may be able to generalize after this has
-	# settled a bit more. It would also be nice to move request-specific
-	# state out of this module -- we could for example redef Request in
-	# main.zeek as needed.
-
-	# State specific to the set_configuration request/response events
-	type SetConfigurationState: record {
-		config: ClusterController::Types::Configuration;
-		requests: set[string] &default=set();
-	};
-
-	# State specific to supervisor interactions
-	type SupervisorState: record {
-		node: string;
-	};
-
-	# State for testing events
-	type TestState: record {
-	};
-
-	# The redef is a workaround so we can use the Request type
-	# while it is still being defined.
-	redef record Request += {
+		## The results vector builds up the list of results we eventually
+		## send to the requestor when we have processed the request.
 		results: ClusterController::Types::ResultVec &default=vector();
-		finished: bool &default=F;
 
-		set_configuration_state: SetConfigurationState &optional;
-		supervisor_state: SupervisorState &optional;
-		test_state: TestState &optional;
+		## An internal flag to track whether a request is complete.
+		finished: bool &default=F;
 	};
 
 	## A token request that serves as a null/nonexistant request.