From f353ac22a58d456f36dbf48a0f1a14fbe89828bd Mon Sep 17 00:00:00 2001
From: Christian Kreibich <christian@corelight.com>
Date: Wed, 15 Jun 2022 15:35:19 -0700
Subject: [PATCH] Management framework: consistency fixes to the Result record

The instance and error fields are now optional instead of defaulting to empty
strings, which caused minor output deviations in the client.

Agents now ensure that any Result record they create has the instance field
filled in.
---
 .../policy/frameworks/management/agent/main.zeek  |  9 ++++++---
 scripts/policy/frameworks/management/types.zeek   | 15 ++++++++++-----
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/scripts/policy/frameworks/management/agent/main.zeek b/scripts/policy/frameworks/management/agent/main.zeek
index 4757f5a3ec..17b14e9c4d 100644
--- a/scripts/policy/frameworks/management/agent/main.zeek
+++ b/scripts/policy/frameworks/management/agent/main.zeek
@@ -637,9 +637,11 @@ event Management::Agent::API::node_dispatch_request(reqid: string, action: vecto
 			add req$node_dispatch_state_agent$requests[node];
 		else
 			{
-			res = Management::Result($reqid=reqid, $node=node);
-			res$success = F;
-			res$error = fmt("cluster node %s not in runnning state", node);
+			res = Management::Result($reqid=reqid,
+			    $instance = Management::Agent::get_name(),
+			    $success = F,
+			    $error = fmt("cluster node %s not in runnning state", node),
+			    $node=node);
 			req$results += res;
 			}
 		}
@@ -730,6 +732,7 @@ event Management::Node::API::notify_node_hello(node: string)
 event Management::Request::request_expired(req: Management::Request::Request)
 	{
 	local res = Management::Result($reqid=req$id,
+	    $instance = Management::Agent::get_name(),
 	    $success = F,
 	    $error = "request timed out");
 
diff --git a/scripts/policy/frameworks/management/types.zeek b/scripts/policy/frameworks/management/types.zeek
index c4076cd8f7..7632cc0247 100644
--- a/scripts/policy/frameworks/management/types.zeek
+++ b/scripts/policy/frameworks/management/types.zeek
@@ -92,13 +92,18 @@ export {
 
 	type NodeStatusVec: vector of NodeStatus;
 
-	## Return value for request-response API event pairs
+	## Return value for request-response API event pairs. Some responses
+	## contain one, others multiple of these. The request ID allows clients
+	## to string requests and responses together. Agents and the controller
+	## fill in the instance and node fields whenever there's sufficient
+	## context to define them. Any result produced by an agent will carry an
+	## instance value, for example.
 	type Result: record {
 		reqid: string;                ##< Request ID of operation this result refers to
-		instance: string &default=""; ##< Name of associated instance (for context)
 		success: bool &default=T;     ##< True if successful
+		instance: string &optional;   ##< Name of associated instance (for context)
 		data: any &optional;          ##< Addl data returned for successful operation
-		error: string &default="";    ##< Descriptive error on failure
+		error: string &optional;      ##< Descriptive error on failure
 		node: string &optional;       ##< Name of associated node (for context)
 	};
 
@@ -127,7 +132,7 @@ function result_to_string(res: Result): string
 
 	if ( res$success )
 		result = "success";
-	else if ( res$error != "" )
+	else if ( res?$error )
 		result = fmt("error (%s)", res$error);
 	else
 		result = "error";
@@ -136,7 +141,7 @@ function result_to_string(res: Result): string
 
 	if ( res$reqid != "" )
 		details[|details|] = fmt("reqid %s", res$reqid);
-	if ( res$instance != "" )
+	if ( res?$instance )
 		details[|details|] = fmt("instance %s", res$instance);
 	if ( res?$node && res$node != "" )
 		details[|details|] = fmt("node %s", res$node);