From 7db091d42726e4a464ebe613caf7128909484e49 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 27 Mar 2022 15:38:41 +0200 Subject: [PATCH 1/7] Simplify wording of save permissions Use shorter and more consise titles for the save management related permissions. --- packages/lib/users.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/lib/users.js b/packages/lib/users.js index f5f59f1a..bbe9b98f 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -379,7 +379,7 @@ definePermission({ }); definePermission({ name: "core.instance.save.list", - title: "List saves on instance", + title: "List saves", description: "List the saves currently on the instance.", }); definePermission({ @@ -389,22 +389,22 @@ definePermission({ }); definePermission({ name: "core.instance.save.create", - title: "Create new instance save", + title: "Create new save", description: "Create new savegames on instances.", }); definePermission({ name: "core.instance.save.delete", - title: "Delete instance save", + title: "Delete save", description: "Delete savegames on instances.", }); definePermission({ name: "core.instance.save.upload", - title: "Upload save to instance", + title: "Upload save", description: "Upload savegames to instances.", }); definePermission({ name: "core.instance.save.download", - title: "Download save from instance", + title: "Download save", description: "Download savegames from instances.", }); definePermission({ From 0de964ec02ae15baf2d9aa8f81bf0b7be30d9289 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 27 Mar 2022 15:50:42 +0200 Subject: [PATCH 2/7] Reorganise instance save commands Move create, list, upload and dalete save management commands to its own 'instance save' namespace in the command line interface. --- packages/ctl/ctl.js | 24 ++++++++++++++---------- test/integration/clusterio.js | 32 ++++++++++++++++---------------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js index b4cc3e5d..1c651140 100755 --- a/packages/ctl/ctl.js +++ b/packages/ctl/ctl.js @@ -533,8 +533,11 @@ async function loadMapSettings(args) { }; } -instanceCommands.add(new libCommand.Command({ - definition: ["list-saves ", "list saves on an instance", (yargs) => { +const instanceSaveCommands = new libCommand.CommandTree({ + name: "save", alias: ["s"], description: "Instance save management", +}); +instanceSaveCommands.add(new libCommand.Command({ + definition: ["list ", "list saves on an instance", (yargs) => { yargs.positional("instance", { describe: "Instance to list saves on", type: "string" }); }], handler: async function(args, control) { @@ -548,8 +551,8 @@ instanceCommands.add(new libCommand.Command({ }, })); -instanceCommands.add(new libCommand.Command({ - definition: ["create-save [name]", "Create a new save on an instance", (yargs) => { +instanceSaveCommands.add(new libCommand.Command({ + definition: ["create [name]", "Create a new save on an instance", (yargs) => { yargs.positional("instance", { describe: "Instance to create on", type: "string" }); yargs.positional("name", { describe: "Name of save to create.", type: "string", default: "world.zip" }); yargs.options({ @@ -573,8 +576,8 @@ instanceCommands.add(new libCommand.Command({ }, })); -instanceCommands.add(new libCommand.Command({ - definition: ["upload-save ", "Upload a save to an instance", (yargs) => { +instanceSaveCommands.add(new libCommand.Command({ + definition: ["upload ", "Upload a save to an instance", (yargs) => { yargs.positional("instance", { describe: "Instance to upload to", type: "string" }); yargs.positional("filepath", { describe: "Path to save to upload", type: "string" }); yargs.options({ @@ -624,8 +627,8 @@ instanceCommands.add(new libCommand.Command({ }, })); -instanceCommands.add(new libCommand.Command({ - definition: ["download-save ", "Download a save from an instance", (yargs) => { +instanceSaveCommands.add(new libCommand.Command({ + definition: ["download ", "Download a save from an instance", (yargs) => { yargs.positional("instance", { describe: "Instance to download save from", type: "string" }); yargs.positional("save", { describe: "Save to download", type: "string" }); }], @@ -669,8 +672,8 @@ instanceCommands.add(new libCommand.Command({ }, })); -instanceCommands.add(new libCommand.Command({ - definition: ["delete-save ", "Delete a save from an instance", (yargs) => { +instanceSaveCommands.add(new libCommand.Command({ + definition: ["delete ", "Delete a save from an instance", (yargs) => { yargs.positional("instance", { describe: "Instance to delete save from", type: "string" }); yargs.positional("save", { describe: "Save to delete", type: "string" }); }], @@ -682,6 +685,7 @@ instanceCommands.add(new libCommand.Command({ }); }, })); +instanceCommands.add(instanceSaveCommands); instanceCommands.add(new libCommand.Command({ definition: ["export-data ", "Export item icons and locale from instance", (yargs) => { diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index b066d15f..77c51b39 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -250,11 +250,11 @@ describe("Integration of Clusterio", function() { }); }); - describe("instance create-save", function() { + describe("instance save create", function() { it("creates a save", async function() { slowTest(this); getControl().saveListUpdates = []; - await execCtl("instance create-save test"); + await execCtl("instance save create test"); await checkInstanceStatus(44, "stopped"); }); }); @@ -266,10 +266,10 @@ describe("Integration of Clusterio", function() { }); }); - describe("instance list-saves", function() { + describe("instance save list", function() { it("lists the created save", async function() { slowTest(this); - let result = await execCtl("instance list-saves test"); + let result = await execCtl("instance save list test"); assert(/world\.zip/.test(result.stdout), "world.zip not present in list save output"); }); }); @@ -530,10 +530,10 @@ describe("Integration of Clusterio", function() { }); }); - describe("instance upload-save", function() { + describe("instance save upload", function() { it("should upload a zip file", async function() { await fs.outputFile(path.join("temp", "test", "upload.zip"), "a test"); - await execCtl("instance upload-save 44 upload.zip"); + await execCtl("instance save upload 44 upload.zip"); assert( await fs.pathExists(path.join("temp", "test", "instances", "test", "saves", "upload.zip")), "file not uploaded to saves directory" @@ -541,42 +541,42 @@ describe("Integration of Clusterio", function() { }); it("should reject non-zip files", async function() { await fs.outputFile(path.join("temp", "test", "invalid"), "a test"); - await assert.rejects(execCtl("instance upload-save 44 invalid")); + await assert.rejects(execCtl("instance save upload 44 invalid")); }); it("should reject path traversal attacks", async function() { await fs.outputFile(path.join("temp", "test", "upload.zip"), "a test"); - await assert.rejects(execCtl("instance upload-save 44 upload.zip --name ../traversal.zip")); + await assert.rejects(execCtl("instance save upload 44 upload.zip --name ../traversal.zip")); }); }); - describe("instance download-save", function() { + describe("instance save download", function() { it("should download a save", async function() { await fs.remove(path.join("temp", "test", "upload.zip")); - await execCtl("instance download-save 44 upload.zip"); + await execCtl("instance save download 44 upload.zip"); assert(await fs.pathExists(path.join("temp", "test", "upload.zip"))); }); it("should error if save does not exist", async function() { - await assert.rejects(execCtl("instance download-save 44 invalid")); + await assert.rejects(execCtl("instance save download 44 invalid")); }); it("should error on path traversal attacks", async function() { - await assert.rejects(execCtl("instance download-save 44 ../factorio-current.log")); + await assert.rejects(execCtl("instance save download 44 ../factorio-current.log")); }); }); - describe("instance delete-save", function() { + describe("instance save delete", function() { it("should delete a save", async function() { - await execCtl("instance delete-save 44 upload.zip"); + await execCtl("instance save delete 44 upload.zip"); assert( !await fs.pathExists(path.join("temp", "test", "instances", "test", "saves", "upload.zip")), "file not deleted" ); }); it("should error if save does not exist", async function() { - await assert.rejects(execCtl("instance delete-save 44 upload.zip")); + await assert.rejects(execCtl("instance save delete 44 upload.zip")); }); it("should error on path traversal attacks", async function() { await fs.outputFile(path.join("temp", "test", "upload.zip"), "a test"); - await assert.rejects(execCtl("instance delete-save 44 ../../upload.zip")); + await assert.rejects(execCtl("instance save delete 44 ../../upload.zip")); }); }); From 9b5c6f8554a5b68f1c26f82bd33692767372591b Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 27 Mar 2022 18:16:56 +0200 Subject: [PATCH 3/7] Remove duplicated request checks Move duplicated checking of save filenames and instance existing from requests into separate functions. --- packages/slave/slave.js | 62 +++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/packages/slave/slave.js b/packages/slave/slave.js index f4916ff8..cdb506b4 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1077,6 +1077,15 @@ function checkFilename(name) { } } +function checkRequestSaveName(name) { + try { + checkFilename(name); + } catch (err) { + throw new libErrors.RequestError(`Save name ${err.message}`); + } +} + + /** * Handles running the slave * @@ -1351,6 +1360,14 @@ class Slave extends libLink.Link { } } + getRequestInstanceInfo(instanceId) { + let instanceInfo = this.instanceInfos.get(instanceId); + if (!instanceInfo) { + throw new libErrors.RequestError(`Instance with ID ${instanceId} does not exist`); + } + return instanceInfo; + } + /** * Initialize and connect an unloaded instance * @@ -1358,11 +1375,7 @@ class Slave extends libLink.Link { * @returns {module:slave/slave~InstanceConnection} connection to instance. */ async _connectInstance(instanceId) { - let instanceInfo = this.instanceInfos.get(instanceId); - if (!instanceInfo) { - throw new libErrors.RequestError(`Instance with ID ${instanceId} does not exist`); - } - + let instanceInfo = this.getRequestInstanceInfo(instanceId); if (this.instanceConnections.has(instanceId)) { throw new libErrors.RequestError(`Instance with ID ${instanceId} is running`); } @@ -1424,11 +1437,7 @@ class Slave extends libLink.Link { return await request.send(instanceConnection, message.data); } - let instanceInfo = this.instanceInfos.get(instanceId); - if (!instanceInfo) { - throw new libErrors.RequestError(`Instance with ID ${instanceId} does not exist`); - } - + let instanceInfo = this.getRequestInstanceInfo(instanceId); return { list: await Instance.listSaves(path.join(instanceInfo.path, "saves"), null), }; @@ -1454,15 +1463,8 @@ class Slave extends libLink.Link { async deleteSaveRequestHandler(message) { let { instance_id, save } = message.data; - try { - checkFilename(save); - } catch (err) { - throw new libErrors.RequestError(`Save name ${err.message}`); - } - let instanceInfo = this.instanceInfos.get(instance_id); - if (!instanceInfo) { - throw new libErrors.RequestError(`Instance with ID ${instance_id} does not exist`); - } + checkRequestSaveName(save); + let instanceInfo = this.getRequestInstanceInfo(instance_id); try { await fs.unlink(path.join(instanceInfo.path, "saves", save)); @@ -1477,15 +1479,8 @@ class Slave extends libLink.Link { async pullSaveRequestHandler(message) { let { instance_id, stream_id, filename } = message.data; - try { - checkFilename(filename); - } catch (err) { - throw new libErrors.RequestError(`Save name ${err.message}`); - } - let instanceInfo = this.instanceInfos.get(instance_id); - if (!instanceInfo) { - throw new libErrors.RequestError(`Instance with ID ${instance_id} does not exist`); - } + checkRequestSaveName(filename); + let instanceInfo = this.getRequestInstanceInfo(instance_id); let url = new URL(this.config.get("slave.master_url")); url.pathname += `api/stream/${stream_id}`; @@ -1528,15 +1523,8 @@ class Slave extends libLink.Link { async pushSaveRequestHandler(message) { let { instance_id, stream_id, save } = message.data; - try { - checkFilename(save); - } catch (err) { - throw new libErrors.RequestError(`Save name ${err.message}`); - } - let instanceInfo = this.instanceInfos.get(instance_id); - if (!instanceInfo) { - throw new libErrors.RequestError(`Instance with ID ${instance_id} does not exist`); - } + checkRequestSaveName(save); + let instanceInfo = this.getRequestInstanceInfo(instance_id); let content; try { From 4edc7a29492b04eeb7643ca04bbade9f9cabd076 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Sun, 27 Mar 2022 19:53:06 +0200 Subject: [PATCH 4/7] Implement rename and copy save Add ability to rename and copy existing saves on an instance. --- CHANGELOG.md | 2 +- packages/ctl/ctl.js | 32 ++++++++ packages/lib/link/messages.js | 22 +++++ packages/lib/users.js | 10 +++ packages/slave/slave.js | 40 +++++++++ packages/web_ui/src/components/SavesList.jsx | 86 +++++++++++++++++++- test/integration/clusterio.js | 35 ++++++++ 7 files changed, 225 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7844f35f..db398cca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ - Packaged and uploaded to npm again, greatly simplifying distribution and installation of both Clusterio and plugins. - Added centralized logging for the cluster. All logs from both Clusterio and Factorio is stored in a shared log which can be inspected and queried. -- Added remote save management with the ability to list, create, upload, download, delete and load saves for instances. +- Added remote save management with the ability to list, create, rename, copy, upload, download, delete and load saves for instances. ### Features diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js index 1c651140..0f6dbedf 100755 --- a/packages/ctl/ctl.js +++ b/packages/ctl/ctl.js @@ -576,6 +576,38 @@ instanceSaveCommands.add(new libCommand.Command({ }, })); +instanceSaveCommands.add(new libCommand.Command({ + definition: ["rename ", "Rename a save on an instance", (yargs) => { + yargs.positional("instance", { describe: "Instance to rename save on", type: "string" }); + yargs.positional("old-name", { describe: "Old name of save.", type: "string" }); + yargs.positional("new-name", { describe: "New name of save.", type: "string" }); + }], + handler: async function(args, control) { + let instanceId = await libCommand.resolveInstance(control, args.instance); + await libLink.messages.renameSave.send(control, { + instance_id: instanceId, + old_name: args.oldName, + new_name: args.newName, + }); + }, +})); + +instanceSaveCommands.add(new libCommand.Command({ + definition: ["copy ", "Copy a save on an instance", (yargs) => { + yargs.positional("instance", { describe: "Instance to copy save on", type: "string" }); + yargs.positional("source", { describe: "Save to copy.", type: "string" }); + yargs.positional("destination", { describe: "Name of copy.", type: "string" }); + }], + handler: async function(args, control) { + let instanceId = await libCommand.resolveInstance(control, args.instance); + await libLink.messages.copySave.send(control, { + instance_id: instanceId, + source: args.source, + destination: args.destination, + }); + }, +})); + instanceSaveCommands.add(new libCommand.Command({ definition: ["upload ", "Upload a save to an instance", (yargs) => { yargs.positional("instance", { describe: "Instance to upload to", type: "string" }); diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 06135ff4..13f5180a 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -550,6 +550,28 @@ messages.createSave = new Request({ }, }); +messages.renameSave = new Request({ + type: "rename_save", + links: ["control-master", "master-slave"], + permission: "core.instance.save.rename", + forwardTo: "instance", + requestProperties: { + "old_name": { type: "string" }, + "new_name": { type: "string" }, + }, +}); + +messages.copySave = new Request({ + type: "copy_save", + links: ["control-master", "master-slave"], + permission: "core.instance.save.copy", + forwardTo: "instance", + requestProperties: { + "source": { type: "string" }, + "destination": { type: "string" }, + }, +}); + messages.deleteSave = new Request({ type: "delete_save", links: ["control-master", "master-slave"], diff --git a/packages/lib/users.js b/packages/lib/users.js index bbe9b98f..a5539b7c 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -392,6 +392,16 @@ definePermission({ title: "Create new save", description: "Create new savegames on instances.", }); +definePermission({ + name: "core.instance.save.rename", + title: "Rename save", + description: "Rename savegames on instances.", +}); +definePermission({ + name: "core.instance.save.copy", + title: "Copy save", + description: "Create copies of savegames on instances.", +}); definePermission({ name: "core.instance.save.delete", title: "Delete save", diff --git a/packages/slave/slave.js b/packages/slave/slave.js index cdb506b4..61e4f110 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1449,6 +1449,46 @@ class Slave extends libLink.Link { await request.send(instanceConnection, message.data); } + async renameSaveRequestHandler(message) { + let { instance_id, old_name, new_name } = message.data; + checkRequestSaveName(old_name); + checkRequestSaveName(new_name); + let instanceInfo = this.getRequestInstanceInfo(instance_id); + try { + await fs.move( + path.join(instanceInfo.path, "saves", old_name), + path.join(instanceInfo.path, "saves", new_name), + { overwrite: false }, + ); + } catch (err) { + if (err.code === "ENOENT") { + throw new libErrors.RequestError(`${old_name} does not exist`); + } + throw err; + } + await this.sendSaveListUpdate(instance_id, path.join(instanceInfo.path, "saves")); + } + + async copySaveRequestHandler(message) { + let { instance_id, source, destination } = message.data; + checkRequestSaveName(source); + checkRequestSaveName(destination); + let instanceInfo = this.getRequestInstanceInfo(instance_id); + try { + await fs.copy( + path.join(instanceInfo.path, "saves", source), + path.join(instanceInfo.path, "saves", destination), + { overwrite: false, errorOnExist: true }, + ); + } catch (err) { + if (err.code === "ENOENT") { + throw new libErrors.RequestError(`${old_name} does not exist`); + } + throw err; + } + await this.sendSaveListUpdate(instance_id, path.join(instanceInfo.path, "saves")); + } + async sendSaveListUpdate(instance_id, savesDir) { let instanceConnection = this.instanceConnections.get(instance_id); let saveList; diff --git a/packages/web_ui/src/components/SavesList.jsx b/packages/web_ui/src/components/SavesList.jsx index 72c0554a..e63524fa 100644 --- a/packages/web_ui/src/components/SavesList.jsx +++ b/packages/web_ui/src/components/SavesList.jsx @@ -1,5 +1,5 @@ import React, { useContext, useState } from "react"; -import { Button, List, Popconfirm, Progress, Space, Table, Tooltip, Upload } from "antd"; +import { Button, Form, Input, List, Modal, Popconfirm, Progress, Space, Table, Tooltip, Upload } from "antd"; import CaretLeftOutlined from "@ant-design/icons/CaretLeftOutlined"; import LeftOutlined from "@ant-design/icons/LeftOutlined"; @@ -13,6 +13,84 @@ import { useSaves } from "../model/saves"; import { notifyErrorHandler } from "../util/notify"; +function RenameModal(props) { + let control = useContext(ControlContext); + let [visible, setVisible] = useState(false); + let [form] = Form.useForm(); + + return <> + + form.submit()} + onCancel={() => setVisible(false)} + > +
{ + libLink.messages.renameSave.send(control, + { instance_id: props.instanceId, old_name: props.save.name, new_name: values.newName } + ).then(() => { + setVisible(false); + form.resetFields(); + }).catch(notifyErrorHandler("Error renaming save")); + }} + > + + + +
+
+ ; +} + +function CopyModal(props) { + let control = useContext(ControlContext); + let [visible, setVisible] = useState(false); + let [form] = Form.useForm(); + + return <> + + form.submit()} + onCancel={() => setVisible(false)} + > +
{ + libLink.messages.copySave.send(control, + { instance_id: props.instanceId, source: props.save.name, destination: values.newName } + ).then(() => { + setVisible(false); + form.resetFields(); + }).catch(notifyErrorHandler("Error copying save")); + }} + > + + + +
+
+ ; +} + function formatBytes(bytes) { if (bytes === 0) { return "0 Bytes"; @@ -81,6 +159,12 @@ export default function SavesList(props) { ); }} >Load save} + {account.hasPermission("core.instance.save.rename") && } + {account.hasPermission("core.instance.save.copy") && } {account.hasPermission("core.instance.save.download") && + form.submit()} + onCancel={() => setVisible(false)} + destroyOnClose + > +
{ + setVisible(false); + let hide = message.loading("Transferring save...", 0); + libLink.messages.transferSave.send(control, { + instance_id: props.instanceId, + source_save: props.save.name, + target_instance_id: values.targetInstanceId, + target_save: values.transferredName || props.save.name, + copy: values.copy, + }).then(() => { + message.success("Transfer complete"); + form.resetFields(); + }).catch( + notifyErrorHandler("Error transfering save") + ).finally(() => hide()); + }} + > + + + + + + + + + +
+
+ ; +} + function formatBytes(bytes) { if (bytes === 0) { return "0 Bytes"; @@ -179,6 +267,9 @@ export default function SavesList(props) { ); }} >Download} + {account.hasPermission("core.instance.save.transfer") && } {account.hasPermission("core.instance.save.delete") && Date: Tue, 29 Mar 2022 22:55:13 +0200 Subject: [PATCH 7/7] Comment out command logging in tests Don't log commands executed while running tests as these spam the output more than they are useful right now. --- test/integration/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/index.js b/test/integration/index.js index b254e05f..f8c40f46 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -109,7 +109,8 @@ let slaveConfigPath = path.join("temp", "test", "config-slave.json"); let controlConfigPath = path.join("temp", "test", "config-control.json"); async function exec(command, options = {}) { - console.log(command); + // Uncomment to show commands run in tests + // console.log(command); options = { cwd: path.join("temp", "test"), ...options }; return await util.promisify(child_process.exec)(command, options); }