Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bin/kibana-plugin] support KP plugins instead #74604

Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Aug 6, 2020

Related to #56205

The current bin/kibana-plugin CLI only supports installing/managing legacy plugins. This PR updates it to only support Kibana Platform plugins. This change is inline with dropping support for legacy plugin support in 7.10.

Release note:

The bin/kibana-plugin CLI has been updated to work with the new Kibana Platform plugin format instead of the legacy plugin format.

@spalger spalger changed the title [cli/kibana-plugin] support KP plugins instead [bin/kibana-plugin] support KP plugins instead Aug 6, 2020
@spalger spalger marked this pull request as ready for review August 7, 2020 01:48
@spalger spalger requested a review from a team as a code owner August 7, 2020 01:48
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@spalger spalger requested a review from a team August 7, 2020 01:48
@spalger
Copy link
Contributor Author

spalger commented Aug 10, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load bundle size

id value diff baseline
upgradeAssistant 65.2KB -36.0B 65.2KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

Tested locally without any issues 👍 Thanks for the help here!

Comment on lines +74 to +77
kibanaVersion:
typeof manifest.kibanaVersion === 'string' && manifest.kibanaVersion
? manifest.kibanaVersion
: manifest.version,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should only use the kibanaVersion key and ignore the plugin version. I'd prefer we err on the side of explicitness here and force plugins to declare their intention. If kibanaVersion is not specified, we should throw an error.

We should also adjust the logic in here in Core to match. This could be done as a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I based this logic off of that logic but I welcome that change as a follow up.


plugins.push({
id: manifest.id,
stripPrefix: match[1],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change from archivePath to stripPrefix? I think the old name was clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly because archivePath sounds like the location of an archive to me, and as far as I can tell this was used for one thing: stripping a prefix from the paths before writing them to disk. I personally like the new name better.

@@ -17,15 +17,15 @@
* under the License.
*/

import { isOSS } from './is_oss';
import { isOss } from './is_oss';

function isXPack(plugin) {
return /x-pack/.test(plugin);
}

export function errorIfXPackInstall(settings) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need any of this logic anymore? X-Pack cannot be installed or removed from a distributable

Copy link
Contributor Author

@spalger spalger Aug 12, 2020

Choose a reason for hiding this comment

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

Yeah, that's what this logic is intended for. If someone tries to install x-pack this logic will help people know they should download the default distributable.

Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@spalger spalger merged commit a735a9f into elastic:master Aug 12, 2020
@spalger spalger deleted the remove/legacy-plugin-installer-support branch August 12, 2020 23:38
spalger pushed a commit to spalger/kibana that referenced this pull request Aug 12, 2020
* [cli/kibana-plugin] support KP plugins instead

* fix some Logger imports from cli/kibana_keystore

Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	packages/kbn-pm/dist/index.js
spalger added a commit that referenced this pull request Aug 13, 2020
Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 13, 2020
* master: (28 commits)
  [Task manager] Prevents edge case where already running tasks are reschedule every polling interval (elastic#74606)
  [Security Solution] Fix the status of timelines' bulk actions (elastic#74560)
  Data plugin: Suggested enhance pattern (elastic#74505)
  Use jest.useFakeTimers instead of hard coded timeout for tooltip tests. (elastic#74642)
  [Security Solution][lists] Adds tests for exception lists and items part 2 (elastic#74815)
  [Security Solution][Resolver] fix presentation role on edgeline (elastic#74869)
  [Security Solution][Detections] Refactor ML calls for newest ML permissions (elastic#74582)
  [bin/kibana-plugin] support KP plugins instead (elastic#74604)
  Reduce number of indexed fields in index pattern saved object (elastic#74817)
  [reporting] Pass along generic parameters in high-order route handler (elastic#74892)
  Migrated last pieces of legacy fixture code (elastic#74470)
  Empty index patterns page re-design  (elastic#68819)
  [babel] coalese some versions to prevent breaking yarn install (elastic#74864)
  [Dashboard First] Decouple Attribute Service and By Value Embeddables (elastic#74302)
  Revert "[reporting] Pass along generic parameters in high-order route handler" (elastic#74891)
  [reporting] Pass along generic parameters in high-order route handler (elastic#74879)
  [src/dev/build] implement a getBuildNumber() mock (elastic#74881)
  [Enterprise Search] Add solution-level side navigation (elastic#74705)
  [DOCS] Canvas docs 7.9 refresh (elastic#74000)
  [Security Solution][Resolver]Enzyme test related events closing (elastic#74811)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 13, 2020
…le-buffer-with-update-of-same-id

* upstream/master: (37 commits)
  [Task manager] Prevents edge case where already running tasks are reschedule every polling interval (elastic#74606)
  [Security Solution] Fix the status of timelines' bulk actions (elastic#74560)
  Data plugin: Suggested enhance pattern (elastic#74505)
  Use jest.useFakeTimers instead of hard coded timeout for tooltip tests. (elastic#74642)
  [Security Solution][lists] Adds tests for exception lists and items part 2 (elastic#74815)
  [Security Solution][Resolver] fix presentation role on edgeline (elastic#74869)
  [Security Solution][Detections] Refactor ML calls for newest ML permissions (elastic#74582)
  [bin/kibana-plugin] support KP plugins instead (elastic#74604)
  Reduce number of indexed fields in index pattern saved object (elastic#74817)
  [reporting] Pass along generic parameters in high-order route handler (elastic#74892)
  Migrated last pieces of legacy fixture code (elastic#74470)
  Empty index patterns page re-design  (elastic#68819)
  [babel] coalese some versions to prevent breaking yarn install (elastic#74864)
  [Dashboard First] Decouple Attribute Service and By Value Embeddables (elastic#74302)
  Revert "[reporting] Pass along generic parameters in high-order route handler" (elastic#74891)
  [reporting] Pass along generic parameters in high-order route handler (elastic#74879)
  [src/dev/build] implement a getBuildNumber() mock (elastic#74881)
  [Enterprise Search] Add solution-level side navigation (elastic#74705)
  [DOCS] Canvas docs 7.9 refresh (elastic#74000)
  [Security Solution][Resolver]Enzyme test related events closing (elastic#74811)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 13, 2020
* upstream/master: (45 commits)
  [Metrics UI] Fix inventory footer misalignment (elastic#74707)
  Remove legacy optimizer (elastic#73154)
  Update design-specific GH code-owners (elastic#74877)
  skip test Reporting paginates content elastic#74922
  [Metrics UI] Add Jest tests for alert previews (elastic#74890)
  Fixed tooltip (elastic#74074)
  [Ingest Pipelines] Processor forms for processors A-D (elastic#72849)
  [Observability] change ingest manager link (elastic#74928)
  [Task manager] Prevents edge case where already running tasks are reschedule every polling interval (elastic#74606)
  [Security Solution] Fix the status of timelines' bulk actions (elastic#74560)
  Data plugin: Suggested enhance pattern (elastic#74505)
  Use jest.useFakeTimers instead of hard coded timeout for tooltip tests. (elastic#74642)
  [Security Solution][lists] Adds tests for exception lists and items part 2 (elastic#74815)
  [Security Solution][Resolver] fix presentation role on edgeline (elastic#74869)
  [Security Solution][Detections] Refactor ML calls for newest ML permissions (elastic#74582)
  [bin/kibana-plugin] support KP plugins instead (elastic#74604)
  Reduce number of indexed fields in index pattern saved object (elastic#74817)
  [reporting] Pass along generic parameters in high-order route handler (elastic#74892)
  Migrated last pieces of legacy fixture code (elastic#74470)
  Empty index patterns page re-design  (elastic#68819)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants