diff --git a/src/commands/app/install.js b/src/commands/app/install.js index cbdb24b6..8a1a331e 100644 --- a/src/commands/app/install.js +++ b/src/commands/app/install.js @@ -19,7 +19,7 @@ const execa = require('execa') const unzipper = require('unzipper') const { validateJsonWithSchema } = require('../../lib/install-helper') const jsYaml = require('js-yaml') -const { USER_CONFIG_FILE, DEPLOY_CONFIG_FILE } = require('../../lib/defaults') +const { USER_CONFIG_FILE, DEPLOY_CONFIG_FILE, PACKAGE_LOCK_FILE } = require('../../lib/defaults') const ora = require('ora') // eslint-disable-next-line node/no-missing-require @@ -51,7 +51,14 @@ class InstallCommand extends BaseCommand { await this.unzipFile(args.path, outputPath) await this.validateAppConfig(outputPath, USER_CONFIG_FILE) await this.validateDeployConfig(outputPath, DEPLOY_CONFIG_FILE) - await this.npmInstall(flags.verbose) + + const packageLockPath = path.join(outputPath, PACKAGE_LOCK_FILE) + if (fs.existsSync(packageLockPath)) { + await this.npmCI(flags.verbose, flags['allow-scripts']) + } else { + this.warn('No lockfile found, running standard npm install. It is recommended that you include a lockfile with your app bundle.') + await this.npmInstall(flags.verbose, flags['allow-scripts']) + } if (flags.tests) { await this.runTests() } @@ -131,15 +138,26 @@ class InstallCommand extends BaseCommand { } } - async npmInstall (isVerbose) { + async npmInstall (isVerbose = false, allowScripts = true) { + const ignoreScripts = allowScripts ? undefined : '--ignore-scripts' this.spinner.start('Running npm install...') const stdio = isVerbose ? 'inherit' : 'ignore' - return execa('npm', ['install'], { stdio }) + return execa('npm', ['install', ignoreScripts], { stdio }) .then(() => { this.spinner.succeed('Ran npm install') }) } + async npmCI (isVerbose = false, allowScripts = true) { + const ignoreScripts = allowScripts ? undefined : '--ignore-scripts' + this.spinner.start('Running npm ci...') + const stdio = isVerbose ? 'inherit' : 'ignore' + return execa('npm', ['ci', ignoreScripts], { stdio }) + .then(() => { + this.spinner.succeed('Ran npm ci') + }) + } + async runTests (isVerbose) { this.spinner.start('Running app tests...') return this.config.runCommand('app:test').then((result) => { @@ -157,6 +175,11 @@ InstallCommand.description = `This command will support installing apps packaged InstallCommand.flags = { ...BaseCommand.flags, + 'allow-scripts': Flags.boolean({ + description: 'Allow post and preinstall scripts during npm install/npm ci', + default: true, + allowNo: true + }), output: Flags.string({ description: 'The packaged app output folder path', char: 'o', diff --git a/src/commands/app/pack.js b/src/commands/app/pack.js index 608c1de0..89e46e90 100644 --- a/src/commands/app/pack.js +++ b/src/commands/app/pack.js @@ -71,8 +71,13 @@ class Pack extends BaseCommand { this.error(hookResults.failures.map(f => `${f.plugin.name} : ${f.error.message}`).join('\nError: '), { exit: 1 }) } + const filesToInclude = flags['lock-file'] ? ['package-lock.json'] : [] + // 1a. Get file list to pack - const fileList = await this.filesToPack({ filesToExclude: [flags.output, DEFAULTS.DIST_FOLDER, ...distLocations] }) + const fileList = await this.filesToPack({ + filesToExclude: [flags.output, DEFAULTS.DIST_FOLDER, ...distLocations], + filesToInclude + }) this.log('=== Files to pack ===') fileList.forEach((file) => { this.log(` ${file}`) @@ -265,10 +270,11 @@ class Pack extends BaseCommand { * * @param {object} options the options for the method * @param {Array} options.filesToExclude a list of files to exclude + * @param {Array} options.filesToInclude a list of files to include (will always be included) * @param {string} options.workingDirectory the working directory to run `npm pack` in * @returns {Array} a list of files that are to be packed */ - async filesToPack ({ filesToExclude = [], workingDirectory = process.cwd() } = {}) { + async filesToPack ({ filesToExclude = [], filesToInclude = [], workingDirectory = process.cwd() } = {}) { const { stdout } = await execa('npm', ['pack', '--dry-run', '--json'], { cwd: workingDirectory }) const noJunkFiles = (file) => { @@ -290,11 +296,14 @@ class Pack extends BaseCommand { } const { files } = JSON.parse(stdout)[0] - return files - .map(file => file.path) - .filter(file => !filesToExclude.includes(file)) - .filter(noJunkFiles) // no junk files like .DS_Store - .filter(noDotFiles) // no files that start with a '.' + return [ + ...files + .map(file => file.path) + .filter(file => !filesToExclude.includes(file)) + .filter(noJunkFiles) // no junk files like .DS_Store + .filter(noDotFiles), // no files that start with a '.' + ...filesToInclude + ] } /** @@ -361,6 +370,11 @@ Pack.description = `This command will support packaging apps for redistribution. Pack.flags = { ...BaseCommand.flags, + 'lock-file': Flags.boolean({ + description: 'Include the package-lock.json file in the packaged app', + default: true, + allowNo: true + }), output: Flags.string({ description: 'The packaged app output file path', char: 'o', diff --git a/src/lib/defaults.js b/src/lib/defaults.js index a83fac56..c24d5440 100644 --- a/src/lib/defaults.js +++ b/src/lib/defaults.js @@ -33,6 +33,7 @@ module.exports = { IMPORT_CONFIG_FILE: 'config.json', USER_CONFIG_FILE: 'app.config.yaml', DEPLOY_CONFIG_FILE: 'deploy.yaml', + PACKAGE_LOCK_FILE: 'package-lock.json', LEGACY_RUNTIME_MANIFEST: 'manifest.yml', INCLUDE_DIRECTIVE: '$include', APPLICATION_CONFIG_KEY: 'application', diff --git a/test/commands/app/install.test.js b/test/commands/app/install.test.js index daaecf0a..1f0f0b5b 100644 --- a/test/commands/app/install.test.js +++ b/test/commands/app/install.test.js @@ -13,6 +13,7 @@ governing permissions and limitations under the License. const TheCommand = require('../../../src/commands/app/install.js') const BaseCommand = require('../../../src/BaseCommand.js') const fs = require('fs-extra') +const { PACKAGE_LOCK_FILE } = require('../../../src/lib/defaults.js') const unzipper = require('unzipper') const execa = require('execa') const installHelper = require('../../../src/lib/install-helper') @@ -221,7 +222,7 @@ describe('npmInstall', () => { command = new TheCommand() }) - test('success', async () => { + test('success (defaults)', async () => { execa.mockImplementationOnce((cmd, args, options) => { expect(cmd).toEqual('npm') expect(args).toEqual(['install']) @@ -229,8 +230,7 @@ describe('npmInstall', () => { return Promise.resolve({ stdout: '' }) }) - const isVerbose = false - await expect(command.npmInstall(isVerbose)).resolves.toEqual(undefined) + await expect(command.npmInstall()).resolves.toEqual(undefined) }) test('success --verbose', async () => { @@ -245,6 +245,19 @@ describe('npmInstall', () => { await expect(command.npmInstall(isVerbose)).resolves.toEqual(undefined) }) + test('success --verbose --no-allow-scripts', async () => { + execa.mockImplementationOnce((cmd, args, options) => { + expect(cmd).toEqual('npm') + expect(args).toEqual(['install', '--ignore-scripts']) + expect(options.stdio).toEqual('inherit') + return Promise.resolve({ stdout: '' }) + }) + + const isVerbose = true + const allowScripts = false + await expect(command.npmInstall(isVerbose, allowScripts)).resolves.toEqual(undefined) + }) + test('failure', async () => { const errorMessage = 'npm install error' @@ -258,6 +271,63 @@ describe('npmInstall', () => { }) }) +describe('npmCI', () => { + let command + + beforeEach(() => { + execa.mockReset() + command = new TheCommand() + }) + + test('success (defaults)', async () => { + execa.mockImplementationOnce((cmd, args, options) => { + expect(cmd).toEqual('npm') + expect(args).toEqual(['ci']) + expect(options.stdio).toEqual('ignore') + return Promise.resolve({ stdout: '' }) + }) + + await expect(command.npmCI()).resolves.toEqual(undefined) + }) + + test('success --verbose', async () => { + execa.mockImplementationOnce((cmd, args, options) => { + expect(cmd).toEqual('npm') + expect(args).toEqual(['ci']) + expect(options.stdio).toEqual('inherit') + return Promise.resolve({ stdout: '' }) + }) + + const isVerbose = true + await expect(command.npmCI(isVerbose)).resolves.toEqual(undefined) + }) + + test('success --verbose --no-allow-scripts', async () => { + execa.mockImplementationOnce((cmd, args, options) => { + expect(cmd).toEqual('npm') + expect(args).toEqual(['ci', '--ignore-scripts']) + expect(options.stdio).toEqual('inherit') + return Promise.resolve({ stdout: '' }) + }) + + const isVerbose = true + const allowScripts = false + await expect(command.npmCI(isVerbose, allowScripts)).resolves.toEqual(undefined) + }) + + test('failure', async () => { + const errorMessage = 'npm ci error' + + execa.mockImplementationOnce((cmd, args) => { + expect(cmd).toEqual('npm') + expect(args).toEqual(['ci']) + throw new Error(errorMessage) + }) + + await expect(command.npmCI()).rejects.toThrow(errorMessage) + }) +}) + describe('runTests', () => { let command @@ -284,7 +354,11 @@ describe('runTests', () => { }) describe('run', () => { - test('no flags', async () => { + beforeEach(() => { + fs.existsSync.mockReset() + }) + + test('no flags, no lockfile', async () => { const command = new TheCommand() command.argv = ['my-app.zip'] @@ -295,6 +369,7 @@ describe('run', () => { command.validateDeployConfig = jest.fn() command.runTests = jest.fn() command.npmInstall = jest.fn() + command.npmCI = jest.fn() command.error = jest.fn() await command.run() @@ -305,6 +380,36 @@ describe('run', () => { expect(command.validateDeployConfig).toHaveBeenCalledTimes(1) expect(command.runTests).toHaveBeenCalledTimes(1) expect(command.npmInstall).toHaveBeenCalledTimes(1) + expect(command.npmCI).toHaveBeenCalledTimes(0) + expect(command.error).toHaveBeenCalledTimes(0) + }) + + test('no flags, has lockfile', async () => { + const command = new TheCommand() + command.argv = ['my-app.zip'] + + // since we already unit test the methods above, we mock it here + command.validateZipDirectoryStructure = jest.fn() + command.unzipFile = jest.fn() + command.addCodeDownloadAnnotation = jest.fn() + command.validateDeployConfig = jest.fn() + command.runTests = jest.fn() + command.npmInstall = jest.fn() + command.npmCI = jest.fn() + command.error = jest.fn() + + fs.existsSync.mockImplementation((filePath) => filePath === PACKAGE_LOCK_FILE) + + await command.run() + + expect(command.validateZipDirectoryStructure).toHaveBeenCalledTimes(1) + expect(command.unzipFile).toHaveBeenCalledTimes(1) + expect(libConfig.coalesce).toHaveBeenCalledTimes(1) + expect(libConfig.validate).toHaveBeenCalledTimes(1) + expect(command.validateDeployConfig).toHaveBeenCalledTimes(1) + expect(command.runTests).toHaveBeenCalledTimes(1) + expect(command.npmInstall).toHaveBeenCalledTimes(0) + expect(command.npmCI).toHaveBeenCalledTimes(1) expect(command.error).toHaveBeenCalledTimes(0) }) @@ -321,6 +426,7 @@ describe('run', () => { command.addCodeDownloadAnnotation = jest.fn() command.validateDeployConfig = jest.fn() command.npmInstall = jest.fn() + command.npmCI = jest.fn() command.error = jest.fn() command.runTests = jest.fn(() => { throw errorObject }) @@ -351,6 +457,7 @@ describe('run', () => { command.addCodeDownloadAnnotation = jest.fn() command.validateDeployConfig = jest.fn() command.npmInstall = jest.fn() + command.npmCI = jest.fn() command.error = jest.fn() command.runTests = jest.fn(() => { throw new Error(errorMessage) }) @@ -379,6 +486,7 @@ describe('run', () => { command.validateDeployConfig = jest.fn() command.runTests = jest.fn() command.npmInstall = jest.fn() + command.npmCI = jest.fn() command.error = jest.fn() await command.run() @@ -405,6 +513,7 @@ describe('run', () => { command.validateDeployConfig = jest.fn() command.runTests = jest.fn() command.npmInstall = jest.fn() + command.npmCI = jest.fn() command.error = jest.fn() const err = new Error('fake validation error') @@ -452,6 +561,7 @@ describe('run', () => { command.validateDeployConfig = jest.fn() command.runTests = jest.fn() command.npmInstall = jest.fn() + command.npmCI = jest.fn() command.error = jest.fn() await command.run() diff --git a/test/commands/app/pack.test.js b/test/commands/app/pack.test.js index 799943bc..e7ab494b 100644 --- a/test/commands/app/pack.test.js +++ b/test/commands/app/pack.test.js @@ -332,6 +332,27 @@ test('zipHelper', async () => { }) describe('filesToPack', () => { + test('always include specific files', async () => { + const jsonOutput = [{ + files: [ + { path: 'fileA' }, + { path: 'fileB' } + ] + }] + + execa.mockImplementationOnce((cmd, args) => { + expect(cmd).toEqual('npm') + expect(args).toEqual(['pack', '--dry-run', '--json']) + return { stdout: JSON.stringify(jsonOutput, null, 2) } + }) + + const command = new TheCommand() + command.argv = [] + const includeFiles = ['foo.json', 'bar.json'] + const filesToPack = await command.filesToPack({ filesToInclude: includeFiles }) + expect(filesToPack).toEqual(['fileA', 'fileB', ...includeFiles]) + }) + test('nothing filtered', async () => { const jsonOutput = [{ files: [ @@ -515,6 +536,43 @@ describe('run', () => { expect(command.copyPackageFiles).toHaveBeenCalledTimes(1) expect(command.filesToPack).toHaveBeenCalledTimes(1) + expect(command.filesToPack).toHaveBeenCalledWith({ + filesToExclude: expect.any(Array), + filesToInclude: ['package-lock.json'] + }) + expect(command.createDeployYamlFile).toHaveBeenCalledTimes(1) + expect(command.addCodeDownloadAnnotation).toHaveBeenCalledTimes(1) + expect(command.zipHelper).toHaveBeenCalledTimes(1) + const expectedObj = { + artifactsFolder: path.join('dist', 'app-package'), + appConfig: expect.any(Object) + } + expect(runHook).toHaveBeenCalledWith('pre-pack', expectedObj) + expect(runHook).toHaveBeenCalledWith('post-pack', expectedObj) + }) + + test('defaults with --no-lock-file flag', async () => { + mockGetFullConfig.mockImplementation(async () => fixtureJson('pack/1.all.config.json')) + + const command = new TheCommand() + command.argv = ['--no-lock-file'] + + // since we already unit test the methods above, we mock it here + command.copyPackageFiles = jest.fn() + command.filesToPack = jest.fn(() => (['some-file'])) + command.createDeployYamlFile = jest.fn() + command.addCodeDownloadAnnotation = jest.fn() + command.zipHelper = jest.fn() + const runHook = jest.fn() + command.config = { runHook } + await command.run() + + expect(command.copyPackageFiles).toHaveBeenCalledTimes(1) + expect(command.filesToPack).toHaveBeenCalledTimes(1) + expect(command.filesToPack).toHaveBeenCalledWith({ + filesToExclude: expect.any(Array), + filesToInclude: [] + }) expect(command.createDeployYamlFile).toHaveBeenCalledTimes(1) expect(command.addCodeDownloadAnnotation).toHaveBeenCalledTimes(1) expect(command.zipHelper).toHaveBeenCalledTimes(1)