From 8662243ed18e0f6e3fab3aa1e58683d37f4933da Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 21 Feb 2023 21:29:41 +0100 Subject: [PATCH] Show which config entries requires a restart Add a label in the web UI to config entries that require a restart to take effect as this is not obvious to end users. --- packages/lib/config/classes.js | 25 ++++++++++++++++ packages/lib/config/definitions.js | 29 +++++++++++++++++- .../web_ui/src/components/BaseConfigTree.jsx | 30 ++++++++++++++++--- test/lib/config/classes.js | 1 + 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/packages/lib/config/classes.js b/packages/lib/config/classes.js index 0be19776..ac1591b0 100644 --- a/packages/lib/config/classes.js +++ b/packages/lib/config/classes.js @@ -677,6 +677,12 @@ class ConfigGroup { * @param {Array=} item.enum - * Array of all values that are acceptible. Useful for making * enumerated values. + * @param {boolean=} item.restartRequired - + * True if a restart of the entity this config is attached to is + * required for changes to take effect. Informative only, defaults + * to false. + * @param {Array=} item.restartRequiredProps - + * Properties to invert the value of restartRequired for if present. * @param {boolean=} item.optional - * True if this field can be set to null. Defaults to false. * @param {*=} item.initial_value - @@ -706,6 +712,7 @@ class ConfigGroup { // Validate properties let validTypes = ["boolean", "string", "number", "object"]; + // eslint-disable-next-line complexity Object.keys(item).forEach(key => { let value = item[key]; let valueType = basicType(value); @@ -746,6 +753,18 @@ class ConfigGroup { } break; + case "restartRequired": + if (valueType !== "boolean") { + throw new Error("restartRequired must be a boolean"); + } + break; + + case "restartRequiredProps": + if (valueType !== "array") { + throw new Error("restartRequiredProps must be a array"); + } + break; + case "optional": if (valueType !== "boolean") { throw new Error("optional must be a boolean"); @@ -771,9 +790,14 @@ class ConfigGroup { access: this.defaultAccess, optional: false, fullName: `${this.groupName}.${item.name}`, + restartRequired: false, ...item, }; + if (item.restartRequiredProps && item.type !== "object") { + throw new Error("restartRequiredProps is only allowed if type is object"); + } + if (!computed.access) { throw new Error(`access not set and no defaultAccess defined for ${this.name}`); } @@ -815,6 +839,7 @@ class PluginConfigGroup extends ConfigGroup { this.define({ name: "load_plugin", title: "Load Plugin", + restartRequired: true, type: "boolean", initial_value: true, }); diff --git a/packages/lib/config/definitions.js b/packages/lib/config/definitions.js index d6605c85..5165d021 100644 --- a/packages/lib/config/definitions.js +++ b/packages/lib/config/definitions.js @@ -37,6 +37,7 @@ MasterGroup.define({ name: "http_port", title: "HTTP Port", description: "Port to listen for HTTP connections on, set to null to not listen for HTTP connections.", + restartRequired: true, type: "number", optional: true, initial_value: 8080, @@ -45,6 +46,7 @@ MasterGroup.define({ name: "https_port", title: "HTTPS Port", description: "Port to listen for HTTPS connection on, set to null to not listen for HTTPS connections.", + restartRequired: true, type: "number", optional: true, }); @@ -52,6 +54,7 @@ MasterGroup.define({ name: "bind_address", title: "Bind Address", description: "IP address to bind the HTTP and HTTPS ports on.", + restartRequired: true, type: "string", optional: true, }); @@ -66,6 +69,7 @@ MasterGroup.define({ name: "tls_certificate", title: "TLS Certificate", description: "Path to the certificate to use for HTTPS.", + restartRequired: true, type: "string", optional: true, }); @@ -73,6 +77,7 @@ MasterGroup.define({ name: "tls_private_key", title: "TLS Private Key", description: "Path to the private key to use for HTTPS.", + restartRequired: true, type: "string", optional: true, }); @@ -84,6 +89,7 @@ MasterGroup.define({ "Secret used to generate and verify authentication tokens. " + "Should be a long string of random letters and numbers. " + "Do not share this.", + restartRequired: true, type: "string", optional: true, }); @@ -184,24 +190,28 @@ SlaveGroup.define({ SlaveGroup.define({ name: "instances_directory", description: "Path to directory to store instances in.", + restartRequired: true, type: "string", initial_value: "instances", }); SlaveGroup.define({ name: "master_url", description: "URL to connect to the master server at", + restartRequired: true, type: "string", initial_value: "http://localhost:8080/", }); SlaveGroup.define({ name: "master_token", description: "Token to authenticate to master server with.", + restartRequired: true, type: "string", initial_value: "enter token here", }); SlaveGroup.define({ name: "tls_ca", description: "Path to Certificate Authority to validate TLS connection to master against.", + restartRequired: true, type: "string", optional: true, }); @@ -244,7 +254,7 @@ InstanceGroup.define({ }); InstanceGroup.define({ name: "id", - description: "ID of the slave", + description: "ID of the instance", type: "number", initial_value: () => Math.random() * 2**31 | 0, }); @@ -272,24 +282,28 @@ FactorioGroup.groupName = "factorio"; FactorioGroup.define({ name: "version", description: "Version of the game to run, use latest to run the latest installed version", + restartRequired: true, type: "string", initial_value: "latest", }); FactorioGroup.define({ name: "game_port", description: "UDP port to run game on, uses a random port if null", + restartRequired: true, type: "number", optional: true, }); FactorioGroup.define({ name: "rcon_port", description: "TCP port to run RCON on, uses a random port if null", + restartRequired: true, type: "number", optional: true, }); FactorioGroup.define({ name: "rcon_password", description: "Password for RCON, randomly generated if null", + restartRequired: true, type: "string", optional: true, }); @@ -304,12 +318,14 @@ FactorioGroup.define({ FactorioGroup.define({ name: "mod_pack", description: "Mod pack to use on this server", + restartRequired: true, type: "number", optional: true, }); FactorioGroup.define({ name: "enable_save_patching", description: "Patch saves with Lua code. Required for Clusterio integrations, lua modules, and most plugins.", + restartRequired: true, type: "boolean", initial_value: true, }); @@ -322,24 +338,34 @@ FactorioGroup.define({ FactorioGroup.define({ name: "enable_authserver_bans", description: "Turn on Factorio.com based multiplayer bans.", + restartRequired: true, type: "boolean", initial_value: false, }); FactorioGroup.define({ name: "settings", description: "Settings overridden in server-settings.json", + restartRequired: true, + restartRequiredProps: [ + "afk_autokick_interval", "allow_commands", "autosave_interval", "autosave_only_on_server", "description", + "ignore_player_limit_for_returning_players", "max_players", "max_upload_slots", + "max_upload_in_kilobytes_per_second", "name", "only_admins_can_pause_the_game", "game_password", + "require_user_verification", "tags", "visibility", + ], type: "object", initial_value: {}, // See create instance handler in master. }); FactorioGroup.define({ name: "verbose_logging", description: "Enable verbose logging on the Factorio server", + restartRequired: true, type: "boolean", initial_value: false, }); FactorioGroup.define({ name: "strip_paths", description: "Strip down instance paths in the log", + restartRequired: true, type: "boolean", initial_value: true, }); @@ -364,6 +390,7 @@ FactorioGroup.define({ FactorioGroup.define({ name: "max_concurrent_commands", description: "Maximum number of RCON commands trasmitted in parallel", + restartRequired: true, type: "number", initial_value: 5, }); diff --git a/packages/web_ui/src/components/BaseConfigTree.jsx b/packages/web_ui/src/components/BaseConfigTree.jsx index 1e75338a..68377fff 100644 --- a/packages/web_ui/src/components/BaseConfigTree.jsx +++ b/packages/web_ui/src/components/BaseConfigTree.jsx @@ -1,5 +1,6 @@ import React, { useEffect, useState } from "react"; -import { Button, Card, Checkbox, Form, Input, InputNumber, Space, Spin, Tree, Typography } from "antd"; +import { Button, Card, Checkbox, Form, Input, InputNumber, Space, Spin, Tooltip, Tree, Typography } from "antd"; +import ReloadOutlined from "@ant-design/icons/ReloadOutlined"; const { Title } = Typography; @@ -87,6 +88,11 @@ export default function BaseConfigTree(props) { } } + let restartTip = ; + let treeData = []; let propsMap = new Map(); for (let [name, GroupClass] of props.ConfigClass.groups) { @@ -110,14 +116,27 @@ export default function BaseConfigTree(props) { }; if (def.type === "object") { - childNode.title = ; + let restartRequiredProps = new Set(def.restartRequiredProps || []); + childNode.title = + {def.title || def.name} + {!def.restartRequiredProps && def.restartRequired && restartTip} + } + tooltip={def.description} + />; for (let prop of Object.keys(value)) { let propPath = `${group.name}.${def.name}.${prop}`; + let restart = Boolean( + def.restartRequiredProps && def.restartRequired ^ restartRequiredProps.has(prop) + ); childNode.children.push({ key: propPath, title: + {`"${prop}"`} + {restart && restartTip} + } rules={[{ validator: (_, fieldValue) => { if (!fieldValue.length) { return Promise.reject(new Error("Will be removed")); @@ -184,7 +203,10 @@ export default function BaseConfigTree(props) { } else { childNode.title = + {def.title || def.name} + {def.restartRequired && restartTip} + } validateStatus={errorFields.has(fieldName) ? "error" : undefined} help={errorFields.get(fieldName)} tooltip={def.description} diff --git a/test/lib/config/classes.js b/test/lib/config/classes.js index 49d5d224..23a2a292 100644 --- a/test/lib/config/classes.js +++ b/test/lib/config/classes.js @@ -113,6 +113,7 @@ describe("lib/config/classes", function() { name: "enum", title: "Enum", description: "Enum thingy", + restartRequired: true, enum: ["a", "b", "c"], optional: true, initial_value: "b",