From 0e21060ea3d1b206aadce89121c25c886fd239e5 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Fri, 22 Jan 2021 12:53:53 +0100
Subject: [PATCH] Allow removing properties with set-prop

Implement removal of properties in ConfigGroup.setProp,
set_instance_config_prop, and the set-prop clusterioctl command.
---
 packages/ctl/ctl.js            | 22 ++++++++++++----------
 packages/lib/config/classes.js |  6 +++++-
 packages/lib/link/messages.js  |  1 +
 test/lib/config/classes.js     |  6 ++++++
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js
index ef444ea0..c58e3bda 100755
--- a/packages/ctl/ctl.js
+++ b/packages/ctl/ctl.js
@@ -150,7 +150,7 @@ instanceConfigCommands.add(new libCommand.Command({
 }));
 
 instanceConfigCommands.add(new libCommand.Command({
-	definition: ["set-prop <instance> <field> <prop> <value>", "Set property of field in instance config", (yargs) => {
+	definition: ["set-prop <instance> <field> <prop> [value]", "Set property of field in instance config", (yargs) => {
 		yargs.positional("instance", { describe: "Instance to set config on", type: "string" });
 		yargs.positional("field", { describe: "Field to set", type: "string" });
 		yargs.positional("prop", { describe: "Property to set", type: "string" });
@@ -158,9 +158,15 @@ instanceConfigCommands.add(new libCommand.Command({
 	}],
 	handler: async function(args, control) {
 		let instanceId = await libCommand.resolveInstance(control, args.instance);
-		let value = args.value;
+		let request = {
+			instance_id: instanceId,
+			field: args.field,
+			prop: args.prop,
+		};
 		try {
-			value = JSON.parse(value);
+			if (args.value !== undefined) {
+				request.value = JSON.parse(args.value);
+			}
 		} catch (err) {
 			// If this is looks like an array, object or string literal
 			// throw the parse error, otherwise assume this is a string.
@@ -176,15 +182,11 @@ instanceConfigCommands.add(new libCommand.Command({
 			// bash + npx       : '""That'\''s a \" quote""'
 			// bash + npx -s sh : "'\"\"That'\\''s a \\\" quote\"\"'"
 			if (/^(\[.*]|{.*}|".*")$/.test(value)) {
-				throw new libErrors.CommandError(`In parsing value '${value}': ${err.message}`);
+				throw new libErrors.CommandError(`In parsing value '${args.value}': ${err.message}`);
 			}
+			request.value = args.value;
 		}
-		await libLink.messages.setInstanceConfigProp.send(control, {
-			instance_id: instanceId,
-			field: args.field,
-			prop: args.prop,
-			value,
-		});
+		await libLink.messages.setInstanceConfigProp.send(control, request);
 	},
 }));
 instanceCommands.add(instanceConfigCommands);
diff --git a/packages/lib/config/classes.js b/packages/lib/config/classes.js
index 38a30e19..df5f2ae2 100644
--- a/packages/lib/config/classes.js
+++ b/packages/lib/config/classes.js
@@ -448,7 +448,11 @@ class ConfigGroup {
 		let prev = this._fields.get(name);
 		let updated = {...prev || {}};
 
-		updated[prop] = value;
+		if (value !== undefined) {
+			updated[prop] = value;
+		} else {
+			delete updated[prop];
+		}
 		this._fields.set(name, updated);
 		if (this._config && !isDeepStrictEqual(updated, prev)) {
 			this._config.emit("fieldChanged", this, name, prev);
diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js
index aea0e040..a68ae538 100644
--- a/packages/lib/link/messages.js
+++ b/packages/lib/link/messages.js
@@ -407,6 +407,7 @@ messages.setInstanceConfigProp = new Request({
 	type: "set_instance_config_prop",
 	links: ["control-master"],
 	permission: "core.instance.update_config",
+	requestRequired: ["instance_id", "field", "prop"],
 	requestProperties: {
 		"instance_id": { type: "integer" },
 		"field": { type: "string" },
diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js
index 48db16ec..e566713e 100644
--- a/test/lib/config/classes.js
+++ b/test/lib/config/classes.js
@@ -397,6 +397,12 @@ describe("lib/config/classes", function() {
 				testInstance.setProp("json", "test", true);
 				assert.deepEqual(testInstance.get("json"), { test: true });
 			});
+
+			it("should unset field if passed undefined", function() {
+				testInstance.set("json", { test: true, extra: "yes" });
+				testInstance.setProp("json", "extra", undefined);
+				assert.deepEqual(testInstance.get("json"), { test: true });
+			});
 		});
 	});