From b2263012a5a332298a256a820c87086c1afb35c7 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Thu, 14 Nov 2024 19:17:19 +0530 Subject: [PATCH] chore: Remove `shelljs` from more `appsmithctl` commands (#37383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR removes more uses of `shelljs` from the `appsmithctl` project, towards completely removing that dependency. Then we should be able to build `appsmithctl` with `esbuild`, and reduce false-positive CVEs being reported from `appsmithctl`'s `node_modules` folder. ## Automation /test sanity ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: d63aa150dff9f8117544219e2f2dfc5816e978a8 > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Thu, 14 Nov 2024 09:21:47 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit ## Release Notes - **New Features** - Enhanced backup and restore processes with improved error handling and logging. - Introduced a new utility function to check if the Supervisor process is running. - **Bug Fixes** - Improved error messages for password retrieval during backup operations. - Refined cleanup processes to ensure resources are released properly. - **Documentation** - Updated test cases for backup functionalities to cover more scenarios and ensure asynchronous handling. - **Chores** - Removed unnecessary dependencies and streamlined logging methods across various scripts. --- .../fs/opt/appsmith/utils/bin/backup.js | 23 ++---- .../fs/opt/appsmith/utils/bin/backup.test.js | 1 - .../fs/opt/appsmith/utils/bin/export_db.js | 73 +++++++------------ .../fs/opt/appsmith/utils/bin/import_db.js | 18 ++--- .../fs/opt/appsmith/utils/bin/mailer.js | 6 +- .../fs/opt/appsmith/utils/bin/restore.js | 16 ++-- .../docker/fs/opt/appsmith/utils/bin/utils.js | 31 +++++--- 7 files changed, 70 insertions(+), 98 deletions(-) diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/backup.js b/deploy/docker/fs/opt/appsmith/utils/bin/backup.js index 6268a5aa276..539d9fedd2d 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/backup.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/backup.js @@ -16,13 +16,7 @@ async function run() { let backupRootPath, archivePath, encryptionPassword; let encryptArchive = false; - try { - await utils.execCommandSilent(["/usr/bin/supervisorctl"]); - } catch (e) { - console.error('Supervisor is not running, exiting.'); - process.exitCode = 1; - return; - } + await utils.ensureSupervisorIsRunning(); try { console.log('Available free space at /appsmith-stacks'); @@ -31,7 +25,7 @@ async function run() { checkAvailableBackupSpace(availSpaceInBytes); - const backupRootPath = await generateBackupRootPath(); + backupRootPath = await generateBackupRootPath(); const backupContentsPath = getBackupContentsPath(backupRootPath, timestamp); await fsPromises.mkdir(backupContentsPath); @@ -51,17 +45,17 @@ async function run() { } await exportDockerEnvFile(backupContentsPath, encryptArchive); - const archivePath = await createFinalArchive(backupRootPath, timestamp); + archivePath = await createFinalArchive(backupRootPath, timestamp); // shell.exec("openssl enc -aes-256-cbc -pbkdf2 -iter 100000 -in " + archivePath + " -out " + archivePath + ".enc"); if (encryptArchive){ const encryptedArchivePath = await encryptBackupArchive(archivePath,encryptionPassword); - logger.backup_info('Finished creating an encrypted a backup archive at ' + encryptedArchivePath); + await logger.backup_info('Finished creating an encrypted a backup archive at ' + encryptedArchivePath); if (archivePath != null) { await fsPromises.rm(archivePath, { recursive: true, force: true }); } } else { - logger.backup_info('Finished creating a backup archive at ' + archivePath); + await logger.backup_info('Finished creating a backup archive at ' + archivePath); console.log('********************************************************* IMPORTANT!!! *************************************************************'); console.log('*** Please ensure you have saved the APPSMITH_ENCRYPTION_SALT and APPSMITH_ENCRYPTION_PASSWORD variables from the docker.env file **') console.log('*** These values are not included in the backup export. **'); @@ -70,7 +64,7 @@ async function run() { await fsPromises.rm(backupRootPath, { recursive: true, force: true }); - logger.backup_info('Finished taking a backup at ' + archivePath); + await logger.backup_info('Finished taking a backup at ' + archivePath); } catch (err) { errorCode = 1; @@ -194,9 +188,8 @@ function getGitRoot(gitRoot) { return gitRoot } -async function generateBackupRootPath() { - const backupRootPath = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'appsmithctl-backup-')); - return backupRootPath +function generateBackupRootPath() { + return fsPromises.mkdtemp(path.join(os.tmpdir(), 'appsmithctl-backup-')); } function getBackupContentsPath(backupRootPath, timestamp) { diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js b/deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js index ddf46775e43..d49e4eff4f5 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/backup.test.js @@ -34,7 +34,6 @@ it('Should not hould throw Error when the available size is >= MIN_REQUIRED_DISK it('Generates t', async () => { os.tmpdir = jest.fn().mockReturnValue('temp/dir'); fsPromises.mkdtemp = jest.fn().mockImplementation((a) => a); - backup.generateBackupRootPath().then((response)=>{console.log(response)}) const res = await backup.generateBackupRootPath() expect(res).toBe('temp/dir/appsmithctl-backup-') }); diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/export_db.js b/deploy/docker/fs/opt/appsmith/utils/bin/export_db.js index 9072479450d..6ef9131c127 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/export_db.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/export_db.js @@ -1,66 +1,49 @@ -// Init function export mongodb -const shell = require('shelljs'); +const fsPromises = require("fs/promises"); const Constants = require('./constants'); const utils = require('./utils'); -function export_database() { +async function exportDatabase() { console.log('export_database ....'); - dbUrl = utils.getDburl(); - shell.mkdir('-p', [Constants.BACKUP_PATH]); - const cmd = `mongodump --uri='${dbUrl}' --archive='${Constants.BACKUP_PATH}/${Constants.DUMP_FILE_NAME}' --gzip`; - shell.exec(cmd); + const dbUrl = utils.getDburl(); + await fsPromises.mkdir(Constants.BACKUP_PATH, { recursive: true }); + await utils.execCommand([ + "mongodump", + "--uri=" + dbUrl, + `--archive=${Constants.BACKUP_PATH}/${Constants.DUMP_FILE_NAME}`, + "--gzip", + ]) console.log('export_database done'); } -function stop_application() { - console.log('stop_application ....'); - shell.exec('/usr/bin/supervisorctl stop backend rts'); - console.log('stop_application done'); -} +async function run() { + let errorCode = 0; -function start_application() { - console.log('start_application ....'); - shell.exec('/usr/bin/supervisorctl start backend rts'); - console.log('start_application done'); -} + await utils.ensureSupervisorIsRunning(); -// Main application workflow -function run() { - let errorCode = 0; try { - check_supervisord_status_cmd = '/usr/bin/supervisorctl >/dev/null 2>&1 '; - shell.exec(check_supervisord_status_cmd, function (code) { - if (code > 0) { - shell.echo('application is not running, starting supervisord'); - shell.exec('/usr/bin/supervisord'); - } - }); - - shell.echo('stop backend & rts application before export database'); - stop_application(); - export_database(); - shell.echo('start backend & rts application after export database'); - shell.echo(); - shell.echo('\033[0;33m++++++++++++++++++++ NOTE ++++++++++++++++++++'); - shell.echo(); - shell.echo( + console.log('stop backend & rts application before export database'); + await utils.stop(["backend", "rts"]); + await exportDatabase(); + console.log('start backend & rts application after export database'); + console.log(); + console.log('\033[0;33m++++++++++++++++++++ NOTE ++++++++++++++++++++'); + console.log(); + console.log( 'Please remember to also copy APPSMITH_ENCRYPTION_SALT and APPSMITH_ENCRYPTION_PASSWORD variables from the docker.env file to the target instance where you intend to import this database dump.', ); - shell.echo(); - shell.echo('++++++++++++++++++++++++++++++++++++++++++++++\033[0m'); - shell.echo(); + console.log(); + console.log('++++++++++++++++++++++++++++++++++++++++++++++\033[0m'); + console.log(); } catch (err) { - shell.echo(err); + console.log(err); errorCode = 1; } finally { - start_application(); + await utils.start(["backend", "rts"]); process.exit(errorCode); } } module.exports = { run, - exportDatabase: export_database, - stopApplication: stop_application, - startApplication: start_application, -}; \ No newline at end of file + exportDatabase, +}; diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/import_db.js b/deploy/docker/fs/opt/appsmith/utils/bin/import_db.js index 0d575e4ff16..0fdf31a087f 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/import_db.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/import_db.js @@ -30,12 +30,12 @@ const main = (forceOption) => { check_supervisord_status_cmd = '/usr/bin/supervisorctl' shell.exec(check_supervisord_status_cmd, function (code) { if (code > 0) { - shell.echo('application is not running, starting supervisord') + console.log('application is not running, starting supervisord') shell.exec('/usr/bin/supervisord') } }) - shell.echo('stop backend & rts application before import database') + console.log('stop backend & rts application before import database') stop_application() const shellCmdResult = shell.exec(`mongo ${process.env.APPSMITH_DB_URL} --quiet --eval "db.getCollectionNames().length"`) const collectionsLen = parseInt(shellCmdResult.stdout.toString().trimEnd()) @@ -44,9 +44,9 @@ const main = (forceOption) => { import_database() return } - shell.echo() - shell.echo('**************************** WARNING ****************************') - shell.echo(`Your target database is not empty, it has data in ${collectionsLen} collections.`) + console.log() + console.log('**************************** WARNING ****************************') + console.log(`Your target database is not empty, it has data in ${collectionsLen} collections.`) const input = readlineSync.question('Importing this DB will erase this data. Are you sure you want to proceed?[Yes/No] ') const answer = input && input.toLocaleUpperCase() if (answer === 'Y' || answer === 'YES') { @@ -55,17 +55,17 @@ const main = (forceOption) => { } else if (answer === 'N' || answer === 'NO') { return } - shell.echo(`Your input is invalid. Please try to run import command again.`) + console.log(`Your input is invalid. Please try to run import command again.`) return } else { import_database() return } } catch (err) { - shell.echo(err) + console.log(err) errorCode = 1 } finally { - shell.echo('start backend & rts application after import database') + console.log('start backend & rts application after import database') start_application() process.exit(errorCode) } @@ -73,4 +73,4 @@ const main = (forceOption) => { module.exports = { runImportDatabase: main, -} \ No newline at end of file +} diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/mailer.js b/deploy/docker/fs/opt/appsmith/utils/bin/mailer.js index 20ed96748ea..32e11127f5f 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/mailer.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/mailer.js @@ -1,7 +1,4 @@ const nodemailer = require('nodemailer'); -const shell = require('shelljs'); - - const Constants = require('./constants'); const utils = require('./utils'); const logger = require('./logger'); @@ -66,10 +63,9 @@ async function sendBackupErrorToAdmins(err, backupTimestamp) { } } catch (err) { await logger.backup_error(err.stack); - return; } } module.exports = { sendBackupErrorToAdmins, -}; \ No newline at end of file +}; diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/restore.js b/deploy/docker/fs/opt/appsmith/utils/bin/restore.js index 780ac3d0050..a88878ccc93 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/restore.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/restore.js @@ -3,8 +3,6 @@ const path = require('path'); const os = require('os'); const readlineSync = require('readline-sync'); -const shell = require('shelljs'); - const utils = require('./utils'); const Constants = require('./constants'); const command_args = process.argv.slice(3); @@ -174,14 +172,10 @@ async function run() { let cleanupArchive = false; let overwriteEncryptionKeys = true; let backupFilePath; - try { - shell.exec('/usr/bin/supervisorctl >/dev/null 2>&1', function (code) { - if (code > 0) { - shell.echo('application is not running, starting supervisord'); - shell.exec('/usr/bin/supervisord'); - } - }); + await utils.ensureSupervisorIsRunning(); + + try { let backupFileName = await getBackupFileName(); if (backupFileName == null) { process.exit(errorCode); @@ -209,7 +203,7 @@ async function run() { console.log('****************************************************************'); console.log('Restoring Appsmith instance from the backup at ' + backupFilePath); - utils.stop(['backend', 'rts']); + await utils.stop(["backend", "rts"]); await restoreDatabase(restoreContentsPath, utils.getDburl()); await restoreDockerEnvFile(restoreContentsPath, backupName, overwriteEncryptionKeys); await restoreGitStorageArchive(restoreContentsPath, backupName); @@ -224,7 +218,7 @@ async function run() { if (cleanupArchive){ await fsPromises.rm(backupFilePath, { force: true }); } - utils.start(['backend', 'rts']); + await utils.start(["backend", "rts"]); process.exit(errorCode); } diff --git a/deploy/docker/fs/opt/appsmith/utils/bin/utils.js b/deploy/docker/fs/opt/appsmith/utils/bin/utils.js index f64dfd9f451..319c3d1a72d 100644 --- a/deploy/docker/fs/opt/appsmith/utils/bin/utils.js +++ b/deploy/docker/fs/opt/appsmith/utils/bin/utils.js @@ -1,4 +1,3 @@ -const shell = require("shelljs"); const fsPromises = require("fs/promises"); const Constants = require("./constants"); const childProcess = require("child_process"); @@ -18,18 +17,25 @@ function showHelp() { console.log("\t--help\t\t\t" + "Show help."); } -function stop(apps) { - const appsStr = apps.join(" "); - console.log("Stopping " + appsStr); - shell.exec("/usr/bin/supervisorctl stop " + appsStr); - console.log("Stopped " + appsStr); +async function ensureSupervisorIsRunning() { + try { + await execCommandSilent(["/usr/bin/supervisorctl"]); + } catch (e) { + console.error('Supervisor is not running, exiting.'); + throw e; + } +} + +async function stop(apps) { + console.log("Stopping", apps); + await execCommand(["/usr/bin/supervisorctl", "stop", ...apps]); + console.log("Stopped", apps); } -function start(apps) { - const appsStr = apps.join(" "); - console.log("Starting " + appsStr); - shell.exec("/usr/bin/supervisorctl start " + appsStr); - console.log("Started " + appsStr); +async function start(apps) { + console.log("Starting", apps); + await execCommand(["/usr/bin/supervisorctl", "start", ...apps]); + console.log("Started", apps); } function getDburl() { @@ -186,6 +192,7 @@ function getDatabaseNameFromMongoURI(uri) { module.exports = { showHelp, + ensureSupervisorIsRunning, start, stop, execCommand, @@ -196,5 +203,5 @@ module.exports = { preprocessMongoDBURI, execCommandSilent, getDatabaseNameFromMongoURI, - getDburl + getDburl, };