[pack installer] various tweaks from PR review

package.json

@@ -135,6 +135,7 @@
     "style-loader": "0.12.3",
     "tar": "2.2.0",
     "url-loader": "0.5.6",
+    "validate-npm-package-name": "2.2.2",
     "webpack": "1.12.1",
     "webpack-directory-name-as-main": "1.0.0",
     "whatwg-fetch": "0.9.0",

src/cli/cli.js

@@ -1,7 +1,7 @@
 import _ from 'lodash';
-
 import pkg from '../utils/package_json';
 import Command from './command';
+import serveCommand from './serve/serve';
 
 let argv = process.env.kbnWorkerArgv ? JSON.parse(process.env.kbnWorkerArgv) : process.argv.slice();
 let program = new Command('bin/kibana');
@@ -14,7 +14,7 @@ program
 );
 
 // attach commands
-require('./serve/serve')(program);
+serveCommand(program);
 
 program
 .command('help <command>')

src/cli_plugin/cli.js

@@ -1,6 +1,9 @@
 import _ from 'lodash';
 import pkg from '../utils/packageJson';
 import Command from '../cli/Command';
+import listCommand from './list';
+import installCommand from './install';
+import removeCommand from './remove';
 
 let argv = process.env.kbnWorkerArgv ? JSON.parse(process.env.kbnWorkerArgv) : process.argv.slice();
 let program = new Command('bin/kibana-plugin');
@@ -12,9 +15,9 @@ program
   'based analytics and search dashboard for Elasticsearch.'
 );
 
-require('./list')(program);
-require('./install')(program);
-require('./remove')(program);
+listCommand(program);
+installCommand(program);
+removeCommand(program);
 
 program
 .command('help <command>')

src/cli_plugin/install/__tests__/download.js

@@ -1,7 +1,7 @@
 import expect from 'expect.js';
 import sinon from 'sinon';
 import nock from 'nock';
-import glob from 'glob';
+import glob from 'glob-all';
 import rimraf from 'rimraf';
 import mkdirp from 'mkdirp';
 import Logger from '../../lib/logger';

src/cli_plugin/install/__tests__/pack.js

@@ -1,6 +1,6 @@
 import expect from 'expect.js';
 import sinon from 'sinon';
-import glob from 'glob';
+import glob from 'glob-all';
 import rimraf from 'rimraf';
 import mkdirp from 'mkdirp';
 import Logger from '../../lib/logger';
@@ -157,6 +157,16 @@ describe('kibana cli', function () {
         });
       });
 
+      it('throw an error if there an invalid plugin name', function () {
+        return copyReplyFile('invalid_name.zip')
+        .then((data) => {
+          return getPackData(settings, logger);
+        })
+        .then(shouldReject, (err) => {
+          expect(err.message).to.match(/invalid plugin name/i);
+        });
+      });
+
     });
 
   });

src/cli_plugin/install/__tests__/zip.js

@@ -1,6 +1,6 @@
 import expect from 'expect.js';
 import sinon from 'sinon';
-import glob from 'glob';
+import glob from 'glob-all';
 import rimraf from 'rimraf';
 import mkdirp from 'mkdirp';
 import Logger from '../../lib/logger';

src/cli_plugin/install/pack.js

@@ -2,6 +2,7 @@ import _ from 'lodash';
 import { listFiles, extractFiles } from './zip';
 import { resolve } from 'path';
 import { sync as rimrafSync } from 'rimraf';
+import validate from 'validate-npm-package-name';
 
 //*****************************************
 //Return a list of package.json files in the archive
@@ -46,6 +47,16 @@ function deletePackageFiles(settings, packages) {
   });
 }
 
+//*****************************************
+//Check the plugin name
+//*****************************************
+function validatePackageName(plugin) {
+  const validation = validate(plugin.name);
+  if (!validation.validForNewPackages) {
+    throw new Error(`Invalid plugin name [${plugin.name}] in package.json`);
+  }
+}
+
 //*****************************************
 //Examine each package.json file to determine the plugin name,
 //version, and platform.
@@ -101,6 +112,7 @@ export async function getPackData(settings, logger) {
   if (packages.length === 0) {
     throw new Error('No kibana plugins found in archive');
   }
+  packages.forEach(validatePackageName);
 
   settings.plugins = packages;
 }

src/cli_plugin/list/__tests__/list.js

@@ -5,6 +5,7 @@ import mkdirp from 'mkdirp';
 import Logger from '../../lib/logger';
 import list from '../list';
 import { join } from 'path';
+import { writeFileSync } from 'fs';
 
 describe('kibana cli', function () {
 
@@ -31,7 +32,7 @@ describe('kibana cli', function () {
       rimraf.sync(pluginDir);
     });
 
-    it('list all of the directories in the plugin folder', function () {
+    it('list all of the folders in the plugin folder', function () {
       mkdirp.sync(join(pluginDir, 'plugin1'));
       mkdirp.sync(join(pluginDir, 'plugin2'));
       mkdirp.sync(join(pluginDir, 'plugin3'));
@@ -56,6 +57,19 @@ describe('kibana cli', function () {
       expect(logger.log.calledWith('.bar')).to.be(false);
     });
 
+    it('list should only list folders', function () {
+      mkdirp.sync(join(pluginDir, 'plugin1'));
+      mkdirp.sync(join(pluginDir, 'plugin2'));
+      mkdirp.sync(join(pluginDir, 'plugin3'));
+      writeFileSync(join(pluginDir, 'plugin4'), 'This is a file, and not a folder.');
+
+      list(settings, logger);
+
+      expect(logger.log.calledWith('plugin1')).to.be(true);
+      expect(logger.log.calledWith('plugin2')).to.be(true);
+      expect(logger.log.calledWith('plugin3')).to.be(true);
+    });
+
   });
 
 });

src/cli_plugin/list/__tests__/settings.js

@@ -1,8 +1,6 @@
 import path from 'path';
 import expect from 'expect.js';
-
-const utils = require('requirefrom')('src/utils');
-const fromRoot = utils('fromRoot');
+import fromRoot from '../../../utils/fromRoot';
 import { resolve } from 'path';
 import { parseMilliseconds, parse } from '../settings';
 

src/cli_plugin/list/list.js

@@ -1,9 +1,12 @@
-import fs from 'fs';
+import { statSync, readdirSync } from 'fs';
+import { join } from 'path';
 
 export default function list(settings, logger) {
-  fs.readdirSync(settings.pluginDir)
+  readdirSync(settings.pluginDir)
   .forEach((filename) => {
-    if (filename[0] !== '.') {
+    const stat = statSync(join(settings.pluginDir, filename));
+
+    if (stat.isDirectory() && filename[0] !== '.') {
       logger.log(filename);
     }
   });

src/cli_plugin/remove/__tests__/remove.js

@@ -1,17 +1,19 @@
 import expect from 'expect.js';
 import sinon from 'sinon';
-import glob from 'glob';
+import glob from 'glob-all';
 import rimraf from 'rimraf';
 import mkdirp from 'mkdirp';
 import Logger from '../../lib/logger';
 import remove from '../remove';
 import { join } from 'path';
+import { writeFileSync } from 'fs';
 
 describe('kibana cli', function () {
 
   describe('plugin lister', function () {
 
     const pluginDir = join(__dirname, '.test.data');
+    let processExitStub;
     let logger;
 
     const settings = {
@@ -19,6 +21,7 @@ describe('kibana cli', function () {
     };
 
     beforeEach(function () {
+      processExitStub = sinon.stub(process, 'exit');
       logger = new Logger(settings);
       sinon.stub(logger, 'log');
       sinon.stub(logger, 'error');
@@ -27,16 +30,27 @@ describe('kibana cli', function () {
     });
 
     afterEach(function () {
+      processExitStub.restore();
       logger.log.restore();
       logger.error.restore();
       rimraf.sync(pluginDir);
     });
 
-    it('log a message if the plugin is not installed.', function () {
+    it('throw an error if the plugin is not installed.', function () {
       settings.pluginPath = join(pluginDir, 'foo');
-      remove(settings, logger);
+      settings.plugin = 'foo';
 
-      expect(logger.log.firstCall.args[0]).to.match(/not installed/i);
+      remove(settings, logger);
+      expect(logger.error.firstCall.args[0]).to.match(/not installed/);
+      expect(process.exit.called).to.be(true);
+    });
+
+    it('throw an error if the specified plugin is not a folder.', function () {
+      writeFileSync(join(pluginDir, 'foo'), 'This is a file, and not a folder.');
+
+      remove(settings, logger);
+      expect(logger.error.firstCall.args[0]).to.match(/not a plugin/);
+      expect(process.exit.called).to.be(true);
     });
 
     it('delete the specified folder.', function () {

src/cli_plugin/remove/__tests__/settings.js

@@ -1,8 +1,6 @@
 import path from 'path';
 import expect from 'expect.js';
-
-const utils = require('requirefrom')('src/utils');
-const fromRoot = utils('fromRoot');
+import fromRoot from '../../../utils/fromRoot';
 import { resolve } from 'path';
 import { parseMilliseconds, parse } from '../settings';
 

src/cli_plugin/remove/remove.js

@@ -1,19 +1,23 @@
-import fs from 'fs';
+import { statSync } from 'fs';
 import rimraf from 'rimraf';
 
 export default function remove(settings, logger) {
   try {
+    let stat;
     try {
-      fs.statSync(settings.pluginPath);
+      stat = statSync(settings.pluginPath);
     } catch (e) {
-      logger.log(`Plugin ${settings.plugin} is not installed`);
-      return;
+      throw new Error(`Plugin [${settings.plugin}] is not installed`);
+    }
+
+    if (!stat.isDirectory()) {
+      throw new Error(`[${settings.plugin}] is not a plugin`);
     }
 
     logger.log(`Removing ${settings.plugin}...`);
     rimraf.sync(settings.pluginPath);
   } catch (err) {
-    logger.error(`Unable to remove plugin "${settings.plugin}" because of error: "${err.message}"`);
+    logger.error(`Unable to remove plugin because of error: "${err.message}"`);
     process.exit(74); // eslint-disable-line no-process-exit
   }
 }