Require config files to be explicitly created (#1040)

The behaviour of creating the config file if it's missing while
convenient causes confusion and problems when the tools are run
in the wrong folder.  They end up creating a shadow installation
and misdirect the user with nonsensical error messages, instead
pointing to the real problem which is that the tool can't find
the configuration needed to actually run.

Require a config file to be present at startup and refuse to run if this
file is missing.  To support the odd use-case of creating the config
when missing a --create-config flag is added. To conveniently generate
a config file to edit out from the "config create" command is added.

packages/controller/README.md

@@ -15,9 +15,12 @@ Common options:
    See the `plugin` command for managing this list.
 
  * `--config <file>` JSON file to use for storing configuration for the controller.
-   Defaults to `config-controller.json` and will be created if it does not exist.
+   Defaults to `config-controller.json` and is required for operation.
    See the `config` command for inspecting and modifying the configuration.
 
+ * `--create-config` If the config file is missing, create a new one and carry on instead of exiting with an error.
+   Alternative to the `config create` command.
+
 
 ### `plugin <command>`
 
@@ -74,6 +77,13 @@ Manage the controller configuration offline.
 This should only be used when the controller is stopped, otherwise the config read might be out of date and config changes will be overwritten when the controller shuts down.
 
 
+#### `config create`
+
+Create a new config file with the default options inserted into if it doesn't already exist.
+
+See also the `--create-config` option.
+
+
 #### `config set <config-entry> [value]`
 
 Set a config entry to the given value.

packages/controller/controller.ts

@@ -299,6 +299,12 @@ async function initialize(): Promise<InitializeParameters> {
 			default: "config-controller.json",
 			type: "string",
 		})
+		.option("create-config", {
+			nargs: 0,
+			describe: "Create config file if it does not exist",
+			default: false,
+			type: "boolean",
+		})
 		.option("plugin-list", {
 			nargs: 1,
 			describe: "File containing list of plugins available with their install path",
@@ -414,27 +420,11 @@ async function initialize(): Promise<InitializeParameters> {
 	lib.registerPluginPermissions(pluginInfos);
 	lib.addPluginConfigFields(pluginInfos);
 
-	const controllerConfigPath = args.config;
-	const controllerConfigLockPath = `${controllerConfigPath}.lock`;
-	if (args.bypassLockFile) {
-		await fs.unlink(controllerConfigLockPath).catch(() => {}); // ignore error, file might not exist
-	}
-
-	let controllerConfig;
-	const controllerConfigLock = new lib.LockFile(controllerConfigLockPath);
-	logger.info(`Loading config from ${controllerConfigPath}`);
-	try {
-		controllerConfig = await lib.ControllerConfig.fromFile("controller", controllerConfigPath);
-
-	} catch (err: any) {
-		if (err.code === "ENOENT") {
-			logger.info("Config not found, initializing new config");
-			controllerConfig = new lib.ControllerConfig("controller", undefined, controllerConfigPath);
-
-		} else {
-			throw new lib.StartupError(`Failed to load ${controllerConfigPath}: ${err.stack ?? err.message ?? err}`);
-		}
-	}
+	const [controllerConfig, controllerConfigLock] = await lib.loadConfigFromArgs(
+		args,
+		lib.ControllerConfig,
+		"controller",
+	);
 
 	controllerConfig.set("controller.version", packageConfig.version); // Allows tracking last loaded version
 

packages/create/create.js

@@ -841,6 +841,7 @@ async function main() {
 	let adminToken = null;
 	if (["standalone", "controller"].includes(answers.mode)) {
 		logger.info("Setting up controller");
+		await execController(["config", "create"]);
 		await execController(["bootstrap", "create-admin", answers.admin]);
 		await execController(["config", "set", "controller.http_port", answers.httpPort]);
 		await execController(["config", "set", "controller.allow_remote_updates", answers.remoteNpm]);
@@ -851,6 +852,7 @@ async function main() {
 
 	if (answers.mode === "standalone") {
 		logger.info("Setting up host");
+		await execHost(["config", "create"]);
 		await execHost(["config", "set", "host.name", "local"]);
 
 		let result = await execHost(["config", "show", "host.id"]);
@@ -871,6 +873,7 @@ async function main() {
 	if (answers.mode === "host") {
 		logger.info("Setting up host");
 		let hostId = JSON.parse(Buffer.from(answers.controllerToken.split(".")[1], "base64")).host;
+		await execHost(["config", "create"]);
 		await execHost(["config", "set", "host.id", hostId]);
 		await execHost(["config", "set", "host.name", answers.hostName]);
 		await execHost(["config", "set", "host.controller_url", answers.controllerUrl]);
@@ -887,6 +890,7 @@ async function main() {
 	}
 
 	if (answers.mode === "ctl") {
+		await execCtl(["config", "create"]);
 		await execCtl(["config", "set", "ctl.controller_url", answers.controllerUrl]);
 		await execCtl(["config", "set", "ctl.controller_token", answers.controllerToken]);
 	}

packages/ctl/README.md

@@ -17,9 +17,12 @@ Common options:
    See the `plugin` command for managing this list.
 
  * `--config <file>` JSON file to use for storing configuration for ctl.
-   Defaults to `config-ctl.json` and will be created if it does not exist.
+   Defaults to `config-ctl.json` and is required for operation.
    See the `config` command for inspecting and modifying the configuration.
 
+ * `--create-config` If the config file is missing, create a new one and carry on instead of exiting with an error.
+   Alternative to the `config create` command.
+
 
 ### `plugin <command>`
 
@@ -76,6 +79,13 @@ Manage the ctl configuration.
 This allows setting the url and token to the controller to manage.
 
 
+#### `config create`
+
+Create a new config file with the default options inserted into if it doesn't already exist.
+
+See also the `--create-config` option.
+
+
 #### `config set <config-entry> [value]`
 
 Set a config entry to the given value.

packages/ctl/ctl.ts

@@ -128,6 +128,8 @@ interface CtlArguments {
 	logLevel: keyof typeof levels,
 	config: string,
 	pluginList: string,
+	bypassLockFile: boolean,
+	createConfig: boolean,
 }
 
 interface InitializeParameters {
@@ -170,6 +172,12 @@ export async function initialize(
 			defaultDescription: "auto",
 			type: "string",
 		})
+		.option("create-config", {
+			nargs: 0,
+			describe: "Create config file if it does not exist",
+			default: false,
+			type: "boolean",
+		})
 		.option("plugin-list", {
 			nargs: 1,
 			describe: "File containing list of plugins available with their install path",
@@ -222,27 +230,11 @@ export async function initialize(
 		.parse(argv) as CtlArguments
 	;
 
-	const ctlConfigPath = args.config;
-	const ctlConfigLockPath = `${ctlConfigPath}.lock`;
-	if (args.bypassLockFile) {
-		await fs.unlink(ctlConfigLockPath).catch(() => {}); // ignore error, file might not exist
-	}
-
-	let ctlConfig;
-	const ctlConfigLock = new lib.LockFile(ctlConfigLockPath);
-	logger.verbose(`Loading config from ${ctlConfigPath}`);
-	try {
-		ctlConfig = await lib.CtlConfig.fromFile("control", ctlConfigPath);
-
-	} catch (err: any) {
-		if (err.code === "ENOENT") {
-			logger.verbose("Config not found, initializing new config");
-			ctlConfig = new lib.CtlConfig("control", undefined, ctlConfigPath);
-
-		} else {
-			throw new lib.StartupError(`Failed to load ${args.config}: ${err.stack ?? err.message ?? err}`);
-		}
-	}
+	const [ctlConfig, ctlConfigLock] = await lib.loadConfigFromArgs(
+		args,
+		lib.CtlConfig,
+		"control",
+	);
 
 	if (args._.length === 0) {
 		parser.showHelp();

packages/host/README.md

@@ -16,9 +16,12 @@ Common options:
    See the `plugin` command for managing this list.
 
  * `--config <file>` JSON file to use for storing configuration for the host.
-   Defaults to `config-host.json` and will be created if it does not exist.
+   Defaults to `config-host.json` and is required for operation.
    See the `config` command for inspecting and modifying the configuration.
 
+ * `--create-config` If the config file is missing, create a new one and carry on instead of exiting with an error.
+   Alternative to the `config create` command.
+
 
 ### `plugin <command>`
 
@@ -75,6 +78,13 @@ Manage the host configuration offline.
 This should only be used when the host is stopped, otherwise the config read might be out of date and config changes will be overwritten when the host shuts down.
 
 
+#### `config create`
+
+Create a new config file with the default options inserted into if it doesn't already exist.
+
+See also the `--create-config` option.
+
+
 #### `config set <config-entry> [value]`
 
 Set a config entry to the given value.

packages/host/host.ts

@@ -104,6 +104,12 @@ async function startHost() {
 			default: "config-host.json",
 			type: "string",
 		})
+		.option("create-config", {
+			nargs: 0,
+			describe: "Create config file if it does not exist",
+			default: false,
+			type: "boolean",
+		})
 		.option("plugin-list", {
 			nargs: 1,
 			describe: "File containing list of plugins available with their install path",
@@ -166,27 +172,11 @@ async function startHost() {
 	lib.registerPluginPermissions(pluginInfos);
 	lib.addPluginConfigFields(pluginInfos);
 
-	const hostConfigPath = args.config;
-	const hostConfigLockPath = `${hostConfigPath}.lock`;
-	if (args.bypassLockFile) {
-		await fs.unlink(hostConfigLockPath).catch(() => {}); // ignore error, file might not exist
-	}
-
-	let hostConfig;
-	const hostConfigLock = new lib.LockFile(hostConfigLockPath);
-	logger.info(`Loading config from ${hostConfigPath}`);
-	try {
-		hostConfig = await lib.HostConfig.fromFile("host", hostConfigPath);
-
-	} catch (err: any) {
-		if (err.code === "ENOENT") {
-			logger.info("Config not found, initializing new config");
-			hostConfig = new lib.HostConfig("host", undefined, hostConfigPath);
-
-		} else {
-			throw new lib.StartupError(`Failed to load ${args.config}: ${err.stack ?? err.message ?? err}`);
-		}
-	}
+	const [hostConfig, hostConfigLock] = await lib.loadConfigFromArgs(
+		args,
+		lib.HostConfig,
+		"host",
+	);
 
 	hostConfig.set("host.version", packageConfig.version); // Allows tracking last loaded version
 

packages/lib/index.ts

@@ -4,6 +4,7 @@
  */
 export * from "./build_mod.js";
 export * from "./src/api.js";
+export * from "./src/bootstrap.js";
 export * from "./src/command.js";
 export * from "./src/config/index.js";
 export * from "./src/data/index.js";

packages/lib/src/bootstrap.ts

@@ -0,0 +1,50 @@
+import fs from "node:fs/promises";
+
+import { logger } from "./logging.js";
+import { LockFile } from "./LockFile.js";
+import type { Config, ConfigLocation } from "./config/classes.js";
+import { StartupError } from "./errors.js";
+
+export async function loadConfigFromArgs<T extends typeof Config<any>>(
+	args: {
+		config: string,
+		bypassLockFile: boolean,
+		createConfig: boolean,
+		_: (string | number)[],
+	},
+	ConfigClass: T,
+	location: ConfigLocation,
+) {
+	const configLockPath = `${args.config}.lock`;
+	if (args.bypassLockFile) {
+		await fs.unlink(configLockPath).catch(() => {}); // ignore error, file might not exist
+	}
+
+	const isCreateCommand = args._[0] === "config" && args._[1] === "create";
+	const create = args.createConfig || isCreateCommand;
+
+	let config;
+	const configLock = new LockFile(configLockPath);
+	const message = `${create ? "Creating" : "Loading"} config at ${args.config}`;
+	if (location === "control" && !create) {
+		// Ctl uses a verbose level here to avoid spamming the console.
+		// A better idea would probably be to set Ctl's default level to warn.
+		logger.verbose(message);
+	} else {
+		logger.info(message);
+	}
+
+	try {
+		config = await ConfigClass.fromFile(location, args.config);
+
+	} catch (err: any) {
+		if (err.code !== "ENOENT" || !create) {
+			throw new StartupError(`Failed to load ${args.config}: ${err.stack ?? err.message ?? err}`);
+		}
+
+		config = new ConfigClass(location, undefined, args.config);
+		await config.save(true);
+	}
+
+	return [config as InstanceType<T>, configLock] as const;
+}

packages/lib/src/config/classes.ts

@@ -460,9 +460,12 @@ export class Config<
 		return this.fromJSON(fields, location, filepath);
 	}
 
-	/** Saves this config to file if there are unsaved changes */
-	async save() {
-		if (!this.dirty) {
+	/**
+	 * Saves this config to file if there are unsaved changes
+	 * @param force - Set to true to save the file even if no changes have been made.
+	 */
+	async save(force = false) {
+		if (!force && !this.dirty) {
 			return;
 		}
 		if (!this.filepath) {

packages/lib/src/shared_commands.ts

@@ -229,6 +229,7 @@ export async function handlePluginCommand(
  */
 export function configCommand(yargs: any) {
 	yargs
+		.command("create", "Create a new config file if not present")
 		.command("set <field> [value]", "Set config field", (yargs: any) => {
 			yargs.options({
 				"stdin": { describe: "read value from stdin", nargs: 0, type: "boolean" },
@@ -258,7 +259,10 @@ export async function handleConfigCommand(
 ) {
 	let command = (args._ as string[])[1];
 
-	if (command === "list") {
+	if (command === "create") {
+		// By the time we get here loadConfigFromArgs will have already created the config.
+
+	} else if (command === "list") {
 		for (const name of Object.keys(instance.constructor.fieldDefinitions)) {
 			print(`${name} ${JSON.stringify(instance.get(name))}`);
 		}

scripts/version-compat-test.js

@@ -248,6 +248,11 @@ async function main() {
 	console.log(`Testing controller ${await packageVersion("controller", options.controller)} `
 		+ `with host ${await packageVersion("host", options.host)}`);
 
+	try {
+		await run("controller", controller, ["config", "create"], controllerDir);
+	} catch {
+		// Ignore failure on previous version where config create does not exist and is not needed.
+	}
 	await run("controller", controller, ["config", "set", "controller.http_port", String(httpPort)], controllerDir);
 	await run("controller", controller, ["bootstrap", "create-admin", "test"], controllerDir);
 	await run("controller", controller, ["bootstrap", "create-ctl-config", "test"], controllerDir);

test/integration/clusterio.js

@@ -212,26 +212,32 @@ describe("Integration of Clusterio", function() {
 				const lockFilePath = `${controllerConfigPath}.lock`;
 				await fs.copyFile(lockFilePath, "temp.lock");
 				try {
-					await execController("run --bypass-lock-file");
+					await execController("run --create-config --bypass-lock-file");
 				} catch (err) {
 					assert.equal(/Error: Server listening failed/.test(err.stderr), true);
 				}
 				await fs.rename("temp.lock", lockFilePath); // Replace the lockfile after it is deleted
 			});
-			it("should refuse to start when no users are loaded", async function() {
+			it("should refuse to start if the config is missing and --create-config is not passed", async function() {
 				const dir = path.join("temp", "test", "empty_controller");
 				await fs.rm(dir, { force: true, recursive: true, maxRetries: 10 });
 				await fs.mkdir(dir, { recursive: true });
 				await assert.rejects(execController("run", { cwd: dir }));
 			});
-			it("should start when no users are loaded if bypass option given", async function() {
+			it("should refuse to start when no users are loaded", async function() {
+				const dir = path.join("temp", "test", "empty_controller");
+				await fs.rm(dir, { force: true, recursive: true, maxRetries: 10 });
+				await fs.mkdir(dir, { recursive: true });
+				await assert.rejects(execController("run --create-config", { cwd: dir }));
+			});
+			it("should start when no users are loaded and bypass options are given", async function() {
 				slowTest(this);
 				const dir = path.join("temp", "test", "empty_controller");
 				await fs.rm(dir, { force: true, recursive: true, maxRetries: 10 });
 				await fs.mkdir(dir, { recursive: true });
 				const child = await spawnNode(
 					"altController",
-					"../../../packages/controller run --no-check-user-count",
+					"../../../packages/controller run --create-config --no-check-user-count",
 					/Started controller/,
 					{ cwd: dir },
 				);

test/integration/index.js

@@ -354,6 +354,7 @@ before(async function() {
 	});
 
 	console.log("Setting Controller Config");
+	await execController("config create");
 	await execController("config set controller.auth_secret TestSecretDoNotUse");
 	await execController("config set controller.http_port 8880");
 	await execController("config set controller.https_port 4443");
@@ -385,6 +386,7 @@ before(async function() {
 
 	const relativeFactorioDir = path.isAbsolute(factorioDir) ? factorioDir : path.join("..", "..", factorioDir);
 	await execCtlProcess("host create-config --id 4 --name host --generate-token");
+	await execHost("config create");
 	await execHost(`config set host.factorio_directory ${relativeFactorioDir}`);
 
 	hostProcess = await spawnNode("host:", "../../packages/host run", /Started host/);
@@ -394,7 +396,7 @@ before(async function() {
 	control = new TestControl(controlConnector);
 	await controlConnector.connect();
 
-	const initArgs = await initializeCtl(`--plugin-list ${pluginListPath} config list`, undefined, true);
+	const initArgs = await initializeCtl(`--plugin-list ${pluginListPath} config create`, undefined, true);
 	control.config = initArgs.ctlConfig;
 	control.hooks = initArgs.ctlHooks;
 

test/lib/bootstrap.test.js

@@ -0,0 +1,97 @@
+import assert from "node:assert/strict";
+import fs from "node:fs/promises";
+import path from "node:path";
+
+import * as lib from "@clusterio/lib";
+
+describe("lib/bootstrap", function() {
+	describe("loadConfigFromArgs()", function() {
+		const folder = path.join("temp", "test", "bootstrap");
+		const configPath = path.join(folder, "sample-config.json");
+		const noentPath = path.join(folder, "noent-config.json");
+		const createPath1 = path.join(folder, "create-config-1.json");
+		const createPath2 = path.join(folder, "create-config-2.json");
+		before(async function() {
+			await fs.mkdir(folder, { recursive: true });
+			await fs.writeFile(
+				configPath,
+				JSON.stringify({
+					"ctl.controller_url": "http://example/foo",
+				}),
+				{ encoding: "utf8" },
+			);
+			await fs.unlink(noentPath).catch(() => {});
+			await fs.unlink(createPath1).catch(() => {});
+			await fs.unlink(createPath2).catch(() => {});
+		});
+		it("should load an existing config", async function() {
+			const [config] = await lib.loadConfigFromArgs(
+				{
+					config: configPath,
+					bypassLockFile: false,
+					createConfig: false,
+					_: ["run"],
+				},
+				lib.CtlConfig,
+				"control",
+			);
+			assert.equal(config.get("ctl.controller_url"), "http://example/foo");
+		});
+		it("should reject if config does not exist", async function() {
+			await assert.rejects(
+				() => lib.loadConfigFromArgs(
+					{
+						config: noentPath,
+						bypassLockFile: false,
+						createConfig: false,
+						_: ["run"],
+					},
+					lib.CtlConfig,
+					"control",
+				),
+				/Error: Failed to load temp.test.bootstrap.noent-config\.json/
+			);
+		});
+		it("should create the config if it does not exist and createConfig is set", async function() {
+			const [config] = await lib.loadConfigFromArgs(
+				{
+					config: createPath1,
+					bypassLockFile: false,
+					createConfig: true,
+					_: ["run"],
+				},
+				lib.CtlConfig,
+				"control",
+			);
+			assert.equal(config.get("ctl.controller_url"), null);
+			await assert.doesNotReject(fs.access(createPath1));
+		});
+		it("should create the config if it does not exist and the command is config create", async function() {
+			const [config] = await lib.loadConfigFromArgs(
+				{
+					config: createPath2,
+					bypassLockFile: false,
+					createConfig: false,
+					_: ["config", "create"],
+				},
+				lib.CtlConfig,
+				"control",
+			);
+			assert.equal(config.get("ctl.controller_url"), null);
+			await assert.doesNotReject(fs.access(createPath2));
+		});
+		it("should use the existing config if it does not exist and createConfig is set", async function() {
+			const [config] = await lib.loadConfigFromArgs(
+				{
+					config: configPath,
+					bypassLockFile: false,
+					createConfig: true,
+					_: ["run"],
+				},
+				lib.CtlConfig,
+				"control",
+			);
+			assert.equal(config.get("ctl.controller_url"), "http://example/foo");
+		});
+	});
+});