Merge pull request #483 from Hornwitser/control-connection-refactor

Control connection refactor

packages/controller/src/ControlConnection.js

@@ -1,7 +1,6 @@
 "use strict";
 const events = require("events");
 const fs = require("fs-extra");
-const jwt = require("jsonwebtoken");
 const path = require("path");
 
 const libConfig = require("@clusterio/lib/config");
@@ -11,7 +10,6 @@ const libHelpers = require("@clusterio/lib/helpers");
 const libLink = require("@clusterio/lib/link");
 const { logFilter, logger } = require("@clusterio/lib/logging");
 const libLoggingUtils = require("@clusterio/lib/logging_utils");
-const libPlugin = require("@clusterio/lib/plugin");
 const libPrometheus = require("@clusterio/lib/prometheus");
 const libUsers = require("@clusterio/lib/users");
 
@@ -146,19 +144,12 @@ class ControlConnection extends BaseConnection {
 		}
 	}
 
-	generateHostToken(hostId) {
-		return jwt.sign(
-			{ aud: "host", host: hostId },
-			Buffer.from(this._controller.config.get("controller.auth_secret"), "base64")
-		);
-	}
-
 	async generateHostTokenRequestHandler(message) {
 		let hostId = message.data.host_id;
 		if (hostId === null) {
 			hostId = Math.random() * 2**31 | 0;
 		}
-		return { token: this.generateHostToken(hostId) };
+		return { token: this._controller.generateHostToken(hostId) };
 	}
 
 	async createHostConfigRequestHandler(message) {
@@ -174,7 +165,7 @@ class ControlConnection extends BaseConnection {
 		}
 		if (message.data.generate_token) {
 			this.user.checkPermission("core.host.generate_token");
-			hostConfig.set("host.controller_token", this.generateHostToken(hostConfig.get("host.id")));
+			hostConfig.set("host.controller_token", this._controller.generateHostToken(hostConfig.get("host.id")));
 		}
 		return { serialized_config: hostConfig.serialize() };
 	}
@@ -196,7 +187,7 @@ class ControlConnection extends BaseConnection {
 		let list = [];
 		for (let instance of this._controller.instances.values()) {
 			list.push({
-				id: instance.config.get("instance.id"),
+				id: instance.id,
 				name: instance.config.get("instance.name"),
 				assigned_host: instance.config.get("instance.assigned_host"),
 				game_port: instance.game_port || null,
@@ -213,10 +204,10 @@ class ControlConnection extends BaseConnection {
 	instanceUpdated(instance) {
 		if (
 			this.instanceSubscriptions.all
-			|| this.instanceSubscriptions.instance_ids.includes(instance.config.get("instance.id"))
+			|| this.instanceSubscriptions.instance_ids.includes(instance.id)
 		) {
 			libLink.messages.instanceUpdate.send(this, {
-				id: instance.config.get("instance.id"),
+				id: instance.id,
 				name: instance.config.get("instance.name"),
 				assigned_host: instance.config.get("instance.assigned_host"),
 				game_port: instance.game_port || null,
@@ -229,55 +220,11 @@ class ControlConnection extends BaseConnection {
 	async createInstanceRequestHandler(message) {
 		let instanceConfig = new libConfig.InstanceConfig("controller");
 		await instanceConfig.load(message.data.serialized_config);
-
-		let instanceId = instanceConfig.get("instance.id");
-		if (this._controller.instances.has(instanceId)) {
-			throw new libErrors.RequestError(`Instance with ID ${instanceId} already exists`);
-		}
-
-		// Add common settings for the Factorio server
-		let settings = {
-			"name": `${this._controller.config.get("controller.name")} - ${instanceConfig.get("instance.name")}`,
-			"description": `Clusterio instance for ${this._controller.config.get("controller.name")}`,
-			"tags": ["clusterio"],
-			"max_players": 0,
-			"visibility": { "public": true, "lan": true },
-			"username": "",
-			"token": "",
-			"game_password": "",
-			"require_user_verification": true,
-			"max_upload_in_kilobytes_per_second": 0,
-			"max_upload_slots": 5,
-			"ignore_player_limit_for_returning_players": false,
-			"allow_commands": "admins-only",
-			"autosave_interval": 10,
-			"autosave_slots": 5,
-			"afk_autokick_interval": 0,
-			"auto_pause": false,
-			"only_admins_can_pause_the_game": true,
-			"autosave_only_on_server": true,
-
-			...instanceConfig.get("factorio.settings"),
-		};
-		instanceConfig.set("factorio.settings", settings);
-
-		let instance = { config: instanceConfig, status: "unassigned" };
-		this._controller.instances.set(instanceId, instance);
-		await libPlugin.invokeHook(this._controller.plugins, "onInstanceStatusChanged", instance, null);
-		this._controller.addInstanceHooks(instance);
+		await this._controller.instanceCreate(instanceConfig);
 	}
 
 	async deleteInstanceRequestHandler(message, request) {
-		let instance = this._controller.getRequestInstance(message.data.instance_id);
-		if (instance.config.get("instance.assigned_host") !== null) {
-			await this.forwardRequestToInstance(message, request);
-		}
-		this._controller.instances.delete(message.data.instance_id);
-
-		let prev = instance.status;
-		instance.status = "deleted";
-		this._controller.instanceUpdated(instance);
-		await libPlugin.invokeHook(this._controller.plugins, "onInstanceStatusChanged", instance, prev);
+		await this._controller.instanceDelete(message.data.instance_id);
 	}
 
 	async getInstanceConfigRequestHandler(message) {
@@ -287,19 +234,6 @@ class ControlConnection extends BaseConnection {
 		};
 	}
 
-	async updateInstanceConfig(instance) {
-		let hostId = instance.config.get("instance.assigned_host");
-		if (hostId !== null) {
-			let connection = this._controller.wsServer.hostConnections.get(hostId);
-			if (connection) {
-				await libLink.messages.assignInstance.send(connection, {
-					instance_id: instance.config.get("instance.id"),
-					serialized_config: instance.config.serialize("host"),
-				});
-			}
-		}
-	}
-
 	async setInstanceConfigFieldRequestHandler(message) {
 		let instance = this._controller.getRequestInstance(message.data.instance_id);
 		if (message.data.field === "instance.assigned_host") {
@@ -312,54 +246,19 @@ class ControlConnection extends BaseConnection {
 		}
 
 		instance.config.set(message.data.field, message.data.value, "control");
-		await this.updateInstanceConfig(instance);
+		await this._controller.instanceConfigUpdated(instance);
 	}
 
 	async setInstanceConfigPropRequestHandler(message) {
 		let instance = this._controller.getRequestInstance(message.data.instance_id);
 		let { field, prop, value } = message.data;
 		instance.config.setProp(field, prop, value, "control");
-		await this.updateInstanceConfig(instance);
+		await this._controller.instanceConfigUpdated(instance);
 	}
 
 	async assignInstanceCommandRequestHandler(message, request) {
 		let { host_id, instance_id } = message.data;
-		let instance = this._controller.getRequestInstance(instance_id);
-
-		// Check if target host is connected
-		let newHostConnection;
-		if (host_id !== null) {
-			newHostConnection = this._controller.wsServer.hostConnections.get(host_id);
-			if (!newHostConnection) {
-				// The case of the host not getting the assign instance message
-				// still have to be handled, so it's not a requirement that the
-				// target host be connected to the controller while doing the
-				// assignment, but it is IMHO a better user experience if this
-				// is the case.
-				throw new libErrors.RequestError("Target host is not connected to the controller");
-			}
-		}
-
-		// Unassign from currently assigned host if it is connected.
-		let currentAssignedHost = instance.config.get("instance.assigned_host");
-		if (currentAssignedHost !== null && host_id !== currentAssignedHost) {
-			let oldHostConnection = this._controller.wsServer.hostConnections.get(currentAssignedHost);
-			if (oldHostConnection && !oldHostConnection.connector.closing) {
-				await libLink.messages.unassignInstance.send(oldHostConnection, { instance_id });
-			}
-		}
-
-		// Assign to target
-		instance.config.set("instance.assigned_host", host_id);
-		if (host_id !== null) {
-			await libLink.messages.assignInstance.send(newHostConnection, {
-				instance_id,
-				serialized_config: instance.config.serialize("host"),
-			});
-		} else {
-			instance.status = "unassigned";
-			this._controller.instanceUpdated(instance);
-		}
+		await this._controller.instanceAssign(instance_id, host_id);
 	}
 
 	async setSaveListSubscriptionsRequestHandler(message) {

packages/controller/src/Controller.js

@@ -5,6 +5,7 @@ const express = require("express");
 const fs = require("fs-extra");
 const http = require("http");
 const https = require("https");
+const jwt = require("jsonwebtoken");
 const path = require("path");
 const stream = require("stream");
 
@@ -20,6 +21,7 @@ const libPrometheus = require("@clusterio/lib/prometheus");
 const { logger } = require("@clusterio/lib/logging");
 
 const HttpCloser = require("./HttpCloser");
+const InstanceInfo = require("./InstanceInfo");
 const metrics = require("./metrics");
 const routes = require("./routes");
 const UserManager = require("./UserManager");
@@ -68,7 +70,7 @@ class Controller {
 
 		/**
 		 * Mapping of instance id to instance info
-		 * @type {Map<number, Object>}
+		 * @type {Map<number, module:controller/src/InstanceInfo>}
 		 */
 		this.instances = null;
 
@@ -379,7 +381,7 @@ class Controller {
 				let instanceConfig = new libConfig.InstanceConfig("controller");
 				await instanceConfig.load(serializedConfig);
 				let status = instanceConfig.get("instance.assigned_host") === null ? "unassigned" : "unknown";
-				let instance = { config: instanceConfig, status };
+				let instance = new InstanceInfo({ config: instanceConfig, status });
 				instances.set(instanceConfig.get("instance.id"), instance);
 			}
 
@@ -543,6 +545,19 @@ class Controller {
 		}
 	}
 
+	/**
+	 * Generate access token for host
+	 *
+	 * @param {number} hostId - ID of host to generate a token for.
+	 * @returns {string} access token for host
+	 */
+	generateHostToken(hostId) {
+		return jwt.sign(
+			{ aud: "host", host: hostId },
+			Buffer.from(this.config.get("controller.auth_secret"), "base64")
+		);
+	}
+
 	hostUpdated(host) {
 		let update = {
 			agent: host.agent,
@@ -577,6 +592,155 @@ class Controller {
 		return instance;
 	}
 
+	/**
+	 * Create a new instance
+	 *
+	 * Adds common Factorio settings to the provided instance config then
+	 * creates an instance using that config in the cluster.
+	 *
+	 * @example
+	 * let instanceConfig = new libConfig.InstanceConfig("controller");
+	 * await instanceConfig.init();
+	 * instanceConfig.set("instance.name", "My instance");
+	 * await controller.createInstance(instanceConfig);
+	 *
+	 * @param {module:lib/config.InstanceConfig} instanceConfig -
+	 *     Config to base newly created instance on.
+	 * @returns {module:controller/src/InstanceInfo} The created instance
+	 */
+	async instanceCreate(instanceConfig) {
+		let instanceId = instanceConfig.get("instance.id");
+		if (this.instances.has(instanceId)) {
+			throw new libErrors.RequestError(`Instance with ID ${instanceId} already exists`);
+		}
+
+		// Add common settings for the Factorio server
+		let settings = {
+			"name": `${this.config.get("controller.name")} - ${instanceConfig.get("instance.name")}`,
+			"description": `Clusterio instance for ${this.config.get("controller.name")}`,
+			"tags": ["clusterio"],
+			"max_players": 0,
+			"visibility": { "public": true, "lan": true },
+			"username": "",
+			"token": "",
+			"game_password": "",
+			"require_user_verification": true,
+			"max_upload_in_kilobytes_per_second": 0,
+			"max_upload_slots": 5,
+			"ignore_player_limit_for_returning_players": false,
+			"allow_commands": "admins-only",
+			"autosave_interval": 10,
+			"autosave_slots": 5,
+			"afk_autokick_interval": 0,
+			"auto_pause": false,
+			"only_admins_can_pause_the_game": true,
+			"autosave_only_on_server": true,
+
+			...instanceConfig.get("factorio.settings"),
+		};
+		instanceConfig.set("factorio.settings", settings);
+
+		let instance = new InstanceInfo({ config: instanceConfig, status: "unassigned" });
+		this.instances.set(instanceId, instance);
+		await libPlugin.invokeHook(this.plugins, "onInstanceStatusChanged", instance, null);
+		this.addInstanceHooks(instance);
+	}
+
+	/**
+	 * Change assigned host of an instance
+	 *
+	 * Unassigns instance from existing host if already assigned to one and
+	 * then assigns it to the given host.  This is the only supported way of
+	 * changing the instance.assigned_host config entry.
+	 *
+	 * Note: this will not transfer any files or saves stored on the host
+	 * the instance was previously assgined to the new one.
+	 *
+	 * @param {number} instanceId - ID of Instance to assign.
+	 * @param {number} hostId - ID of host to assign instance to.
+	 */
+	async instanceAssign(instanceId, hostId) {
+		let instance = this.getRequestInstance(instanceId);
+
+		// Check if target host is connected
+		let newHostConnection;
+		if (hostId !== null) {
+			newHostConnection = this.wsServer.hostConnections.get(hostId);
+			if (!newHostConnection) {
+				// The case of the host not getting the assign instance message
+				// still have to be handled, so it's not a requirement that the
+				// target host be connected to the controller while doing the
+				// assignment, but it is IMHO a better user experience if this
+				// is the case.
+				throw new libErrors.RequestError("Target host is not connected to the controller");
+			}
+		}
+
+		// Unassign from currently assigned host if it is connected.
+		let currentAssignedHost = instance.config.get("instance.assigned_host");
+		if (currentAssignedHost !== null && hostId !== currentAssignedHost) {
+			let oldHostConnection = this.wsServer.hostConnections.get(currentAssignedHost);
+			if (oldHostConnection && !oldHostConnection.connector.closing) {
+				await libLink.messages.unassignInstance.send(oldHostConnection, { instance_id: instanceId });
+			}
+		}
+
+		// Assign to target
+		instance.config.set("instance.assigned_host", hostId);
+		if (hostId !== null) {
+			await libLink.messages.assignInstance.send(newHostConnection, {
+				instance_id: instanceId,
+				serialized_config: instance.config.serialize("host"),
+			});
+		} else {
+			instance.status = "unassigned";
+			this.instanceUpdated(instance);
+		}
+	}
+
+	/**
+	 * Delete an instance
+	 *
+	 * Permanently deletes the instance from its assigned host (if assigned)
+	 * and the controller.  This action cannot be undone.
+	 *
+	 * @param {number} instanceId - ID of instance to delete.
+	 */
+	async instanceDelete(instanceId) {
+		let instance = this.getRequestInstance(instanceId);
+		if (instance.config.get("instance.assigned_host") !== null) {
+			await this.forwardRequestToInstance(libLink.messages.deleteInstance, { instance_id: instanceId });
+		}
+		this.instances.delete(instanceId);
+
+		let prev = instance.status;
+		instance.status = "deleted";
+		this.instanceUpdated(instance);
+		await libPlugin.invokeHook(this.plugins, "onInstanceStatusChanged", instance, prev);
+	}
+
+	/**
+	 * Notify the config of an instance updated
+	 *
+	 * Used to push config changes to the assigned host when the config of
+	 * an instance has changed.
+	 *
+	 * @param {module:controller/src/InstanceInfo} instance -
+	 *     Instance to nodify the config updated.
+	 */
+	async instanceConfigUpdated(instance) {
+		let hostId = instance.config.get("instance.assigned_host");
+		if (hostId !== null) {
+			let connection = this.wsServer.hostConnections.get(hostId);
+			if (connection) {
+				await libLink.messages.assignInstance.send(connection, {
+					instance_id: instance.id,
+					serialized_config: instance.config.serialize("host"),
+				});
+			}
+		}
+	}
+
 	addInstanceHooks(instance) {
 		instance.config.on("fieldChanged", (group, field, prev) => {
 			if (group.name === "instance" && field === "name") {

packages/controller/src/HostConnection.js

@@ -6,6 +6,7 @@ const libPlugin = require("@clusterio/lib/plugin");
 const PlayerStats = require("@clusterio/lib/PlayerStats");
 
 const BaseConnection = require("./BaseConnection");
+const InstanceInfo = require("./InstanceInfo");
 
 
 /**
@@ -125,7 +126,7 @@ class HostConnection extends BaseConnection {
 		for (let instance of this._controller.instances.values()) {
 			if (instance.config.get("instance.assigned_host") === this._id) {
 				await libLink.messages.assignInstance.send(this, {
-					instance_id: instance.config.get("instance.id"),
+					instance_id: instance.id,
 					serialized_config: instance.config.serialize("host"),
 				});
 			}
@@ -141,7 +142,7 @@ class HostConnection extends BaseConnection {
 				// Check if this instance is assigned somewhere else.
 				if (controllerInstance.config.get("instance.assigned_host") !== this._id) {
 					await libLink.messages.unassignInstance.send(this, {
-						instance_id: controllerInstance.config.get("instance.id"),
+						instance_id: controllerInstance.id,
 					});
 					continue;
 				}
@@ -160,7 +161,7 @@ class HostConnection extends BaseConnection {
 			}
 
 			instanceConfig.set("instance.assigned_host", this._id);
-			let newInstance = { config: instanceConfig, status: instanceData.status };
+			let newInstance = new InstanceInfo({ config: instanceConfig, status: instanceData.status });
 			this._controller.instances.set(instanceConfig.get("instance.id"), newInstance);
 			this._controller.addInstanceHooks(newInstance);
 			await libLink.messages.assignInstance.send(this, {

packages/controller/src/InstanceInfo.js

@@ -0,0 +1,49 @@
+"use strict";
+
+/**
+ * Runtime status of an instance on the controller
+ * @alias module:controller/src/InstanceInfo
+ */
+class InstanceInfo {
+	/** @type {module:lib/config.InstanceConfig} */
+	config;
+
+	/**
+	 * Current status of the instance. One of:
+	 * - `unknown`: Instance is assigned to a host but this host is currently
+	 *   not connected to the contreller.
+	 * - `unassigned`: Instance is not assigned to a a host and exists only on
+	 *   the controller.
+	 * - `stopped`: Instance is stopped.
+	 * - `starting`: Instance is in the process of starting up.
+	 * - `running`: Instance is running normally.
+	 * - `stopping`: Instance is in the process of stopping.
+	 * - `creating_save`: Instance is in the process of creating a save.
+	 * - `exporting_data`: Instance is in the process of exporting game data.
+	 * - `deleted`: Instance has been deleted.
+	 *
+	 * @type {string}
+	 */
+	status;
+
+	// This is not actually json as a config can't be loaded synchronously but
+	// that is likely to change in the future.
+	constructor(json) {
+		this.config = json["config"];
+		this.status = json["status"];
+	}
+
+	toJSON() {
+		return {
+			config: this.config.serialize(),
+			status: this.status,
+		};
+	}
+
+	/** Shorthand for `instance.config.get("instance.id")` @type {number} */
+	get id() {
+		return this.config.get("instance.id");
+	}
+}
+
+module.exports = InstanceInfo;

packages/lib/plugin.js

@@ -354,7 +354,8 @@ class BaseControllerPlugin {
 	 * the case of network outages if reconnect fails to re-establish the
 	 * session between the controller and the host.
 	 *
-	 * @param {Object} instance - the instance that changed.
+	 * @param {module:controller/src/InstanceInfo} instance -
+	 *     The instance that changed.
 	 * @param {?string} prev - the previous status of the instance.
 	 */
 	async onInstanceStatusChanged(instance, prev) { }
@@ -378,7 +379,8 @@ class BaseControllerPlugin {
 	 * Invoked after the value of the config field given by `field` has
 	 * changed on an instance.
 	 *
-	 * @param {Object} instance - The instance the config changed on.
+	 * @param {module:controller/src/InstanceInfo} instance -
+	 *     The instance the config changed on.
 	 * @param {module:lib/config.ConfigGroup} group -
 	 *     The group who's field got changed.
 	 * @param {string} field - Name of the field that changed.
@@ -524,7 +526,8 @@ class BaseControllerPlugin {
 	 * Invoked when a player either joins or leaves an instance in the
 	 * cluster.
 	 *
-	 * @param {Object} instance - the instance it occured on.
+	 * @param {module:controller/src/InstanceInfo} instance -
+	 *     The instance it occured on.
 	 * @param {Object} event - Information about the event.
 	 * @param {string} event.type - Either "join", "leave" or "import".
 	 * @param {string} event.name - Name of the player that joined/left.

plugins/inventory_sync/controller.js

@@ -42,7 +42,7 @@ class ControllerPlugin extends libPlugin.BaseControllerPlugin {
 	}
 
 	async onInstanceStatusChanged(instance) {
-		let instanceId = instance.config.get("instance.id");
+		let instanceId = instance.id;
 		if (["unassigned", "deleted"].includes(instance.status)) {
 			for (let [playerName, acquisitionRecord] of this.acquiredPlayers) {
 				if (acquisitionRecord.instanceId === instanceId) {