Plugin pack installer by BigFunger · Pull Request #6402 · elastic/kibana

BigFunger · 2016-03-03T19:56:18Z

resolves #5761

Plugin pack installer for kibana

modified existing entry point

Removed the plugin commands from the ./bin/kibana entry point

new entry point

All plugin commands are now executed from the ./bin/kibana-plugin entry point:

list  [options]                 List installed plugins
install  [options] <plugin/url> Install a plugin
remove  [options] <plugin>      Remove a plugin
help  <command>                 Get the help for a specific command

List command changes

Refactored to use its own settings object/parser
Now ignores folders that begin with a period.
Added tests

Remove command changes

Refactored to use its own settings object/parser

Install command changes

Refactored to use its own settings object/parser
No longer supports tar.gz format. Only supports zip format
Attempts to download the specified plugin from:
- exact <plugin> specified
- https://download.elastic.co/packs/<plugin>/<plugin>-<kibana version>.zip
Expects the pack zip file if the plugin name is 'xpack' to be structured:

any-named-zip-archive.zip
|__kibana
| |__xpack
| | |__package.json
| | |__...

Expects the package.json file to contain a name (xpack) that matches the containing folder in the archive exactly and a version that matches the version of kibana exactly.
Currently there is no platform-specific logic implemented
Currently will only install the first plugin that it finds in the kibana folder if more are in the archive.
Logic has been added to make implement platform specific and multiple plugins easier for later versions.

w33ble · 2016-03-03T21:15:15Z

Initial observations...

remove and list commands should be checking to ensure that they're dealing with directories, currently they list and remove files
remove doesn't work when folder names have a space
related: install should probably validate the name in the package.json and fail if it contains a space - npm packages can't have a space in their name anyway

w33ble · 2016-03-03T21:22:30Z

Plugin installation was unsuccessful due to error "Incorrect version in plugin [shield]. Expected [5.0.0-snapshot]; found [5.0.0-SNAPSHOT]"

I wonder if we should normalize the version string coming in and cast to lowercase. I also wonder if we should just drop anything after the last number, like we do in the ES version checker.

w33ble · 2016-03-03T21:26:14Z

src/cli_plugin/install/__tests__/download.js

Should be glob-all here

w33ble · 2016-03-03T21:33:28Z

When running help on a command that doesn't exist (ie. kibana-plugin help funger), there's a stray undefined in the output that should be removed.

I think it's trying to show the default usage syntax but failing.

w33ble · 2016-03-03T21:47:54Z

src/cli_plugin/cli.js

I believe with the move to ES6 modules, we've gotten rid of require statements on client and server code now. If I'm right, you should be using import instead of require here, and elsewhere.

w33ble · 2016-03-03T23:06:49Z

The file permissions on bin/kibana-plugin need to include the +x flag. I have no idea how to achieve that on Windows though...

BigFunger · 2016-03-04T20:22:52Z

jenkins, test it

epixa · 2016-03-10T01:22:57Z

src/cli_plugin/cli.js

+import removeCommand from './remove';
+
+let argv = process.env.kbnWorkerArgv ? JSON.parse(process.env.kbnWorkerArgv) : process.argv.slice();
+let program = new Command('bin/kibana-plugin');


I think this should be const

The focus of my changes on this file were removing existing functionality from it. I didn't rewrite this file, so I didn't focus on it as a whole. Despite this, I'll make your suggested change.

epixa · 2016-03-10T14:36:58Z

One thing I wanted to note: the size of this pull request makes it really difficult to review effectively. You broke up the PR description into 5 different categories of changes - I'd recommend 5 different pull requests in the future.

BigFunger · 2016-03-18T16:49:53Z

@epixa said

Rather than error if the user doesn't specify a command in help, I think we should render the default text. Basically, I'd expect both of these to be valid ways to retrieve help text about the cli:

This will require hacking on the Command module we use for the cli. Opened #6577 to address this.

w33ble · 2016-03-23T20:00:21Z

src/cli_plugin/cli.js

@@ -0,0 +1,43 @@
+import _ from 'lodash';
+import pkg from '../utils/package_json';
+import Command from '../cli/Command';


This has to be lowercase - ../cli/command

w33ble · 2016-03-23T20:17:53Z

With that last fix, the installer is now fully functional. LGTM!

epixa · 2016-03-23T22:56:44Z

Is it intentional to allow people to install over existing plugins (without removing them first)?

epixa · 2016-03-23T22:57:14Z

Err, nevermind. It failed eventually.

epixa · 2016-03-23T22:57:59Z

LGTM

BigFunger added the review label Mar 3, 2016

BigFunger assigned w33ble Mar 3, 2016

w33ble reviewed Mar 3, 2016
View reviewed changes

src/cli_plugin/install/__tests__/download.js Outdated

Copy link
Copy Markdown

Contributor

w33ble Mar 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be glob-all here

w33ble reviewed Mar 3, 2016
View reviewed changes

w33ble assigned BigFunger and unassigned w33ble Mar 4, 2016

w33ble added needs updates and removed review labels Mar 4, 2016

BigFunger force-pushed the plugin-pack-installer branch from 29cbbd7 to 91cce98 Compare March 7, 2016 20:42

BigFunger assigned epixa and unassigned BigFunger Mar 7, 2016

BigFunger added review and removed needs updates labels Mar 7, 2016

epixa reviewed Mar 10, 2016
View reviewed changes

BigFunger added 2 commits March 18, 2016 10:08

[pack installer] fixed test

75598c5

[pack installer] changed version checking to use semver

d65e7be

BigFunger force-pushed the plugin-pack-installer branch from 2307095 to d65e7be Compare March 18, 2016 16:13

chmod +x

f24c3b7

BigFunger mentioned this pull request Mar 18, 2016

[kibana cli] expected help on 'kibana help' or 'kibana-plugin help' #6577

Closed

BigFunger added 3 commits March 18, 2016 14:43

[pack installer] fixed version checking

f701df5

Merge branch 'master' into plugin-pack-installer

c0b1b9a

[pack installer] fixed path

cbfcd56

BigFunger force-pushed the plugin-pack-installer branch from f9b46b0 to cbfcd56 Compare March 22, 2016 16:19

Merge branch 'master' into plugin-pack-installer

52f653a

w33ble reviewed Mar 23, 2016
View reviewed changes

[pack installer] fixed case on import statement

dcf1dac

BigFunger assigned epixa and unassigned BigFunger Mar 23, 2016

w33ble added review and removed needs updates labels Mar 23, 2016

epixa added v5.0.0 v5.0.0-alpha1 labels Mar 23, 2016

epixa assigned BigFunger and unassigned epixa Mar 23, 2016

BigFunger merged commit c649b49 into elastic:master Mar 24, 2016

epixa added v5.0.0-alpha1 and removed v5.0.0-alpha1 labels Mar 31, 2016

Bargs added the release_note:breaking label Sep 15, 2016

Conversation

BigFunger commented Mar 3, 2016

Plugin pack installer for kibana

modified existing entry point

new entry point

List command changes

Remove command changes

Install command changes

Uh oh!

w33ble commented Mar 3, 2016

Uh oh!

w33ble commented Mar 3, 2016

Uh oh!

w33ble Mar 3, 2016

Choose a reason for hiding this comment

Uh oh!

w33ble commented Mar 3, 2016

Uh oh!

w33ble Mar 3, 2016

Choose a reason for hiding this comment

Uh oh!

w33ble commented Mar 3, 2016

Uh oh!

BigFunger commented Mar 4, 2016

Uh oh!

epixa Mar 10, 2016

Choose a reason for hiding this comment

Uh oh!

BigFunger Mar 11, 2016

Choose a reason for hiding this comment

Uh oh!

epixa commented Mar 10, 2016

Uh oh!

BigFunger commented Mar 18, 2016

Uh oh!

w33ble Mar 23, 2016

Choose a reason for hiding this comment

Uh oh!

w33ble commented Mar 23, 2016

Uh oh!

epixa commented Mar 23, 2016

Uh oh!

epixa commented Mar 23, 2016

Uh oh!

epixa commented Mar 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants