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

[plugin-helpers] improve 3rd party KP plugin support #75019

Merged
merged 15 commits into from Aug 27, 2020

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Aug 14, 2020

Closes #63069
Closes #70412
Closes #61731

This PR updates the plugin-helpers to be compatible with KP plugins instead of legacy plugins. It drastically refactors the code to both modernize it, improve it's type safety, and remove legacy cruft.

The dev and test:server tasks were removed, along with the old commander based interface. There is now a single command available from the plugin-helpers, build.

The old js API was also removed in favor of existing APIs from packages like @kbn/dev-utils.

The build command creates distributable versions of plugins in a very similar way as it used to, except that those plugins now include a kibana.json file and have browser bundles which are created by the @kbn/optimizer. Additionally, server code is pre-processed by babel allowing us to remove babel support from production completely.

A docs PR will follow with a generation-to-distribution example of developing a 3rd party plugin.

@spalger spalger added Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 14, 2020
@spalger spalger force-pushed the implement/plugin-helpers-kp-support branch 22 times, most recently from 37b6ea0 to b8e932d Compare August 24, 2020 19:08
@spalger spalger force-pushed the implement/plugin-helpers-kp-support branch from b8e932d to f0c0768 Compare August 24, 2020 19:37
@spalger spalger marked this pull request as ready for review August 24, 2020 20:14
Copy link
Contributor

@tylersmalley tylersmalley 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 64311d3 into elastic:master Aug 27, 2020
@spalger spalger deleted the implement/plugin-helpers-kp-support branch August 27, 2020 21:56
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

distributable file count

id value diff baseline
total 44361 -8765 53126

History

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

spalger pushed a commit to spalger/kibana that referenced this pull request Aug 27, 2020
Co-authored-by: Tyler Smalley <tylersmalley@me.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
# Conflicts:
#	packages/kbn-plugin-helpers/package.json
#	x-pack/scripts/functional_tests.js
#	yarn.lock
spalger added a commit that referenced this pull request Aug 27, 2020
…76209)

Co-authored-by: Tyler Smalley <tylersmalley@me.com>
Co-authored-by: spalger <spalger@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 31, 2020
…s-for-710

* 'master' of github.com:elastic/kibana: (43 commits)
  [APM] Chart units don't update when toggling the chart legends (elastic#74931)
  [ILM] Add support for frozen phase in UI (elastic#75968)
  Hides advanced json for count metric (elastic#74636)
  add client-side feature usage API (elastic#75486)
  [Maps] add drilldown support map embeddable (elastic#75598)
  [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106)
  [Resolver] model `location.search` in redux (elastic#76140)
  [APM] Prevent imports of public in server code (elastic#75979)
  fix eslint issue
  skip flaky suite (elastic#76223)
  [APM] Transaction duration anomaly alerting integration (elastic#75719)
  [Transforms] Avoid using "Are you sure" (elastic#75932)
  [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145)
  [plugin-helpers] improve 3rd party KP plugin support (elastic#75019)
  [docs/getting-started] link to yarn v1 specifically (elastic#76169)
  [Security_Solution][Resolver] Resolver loading and error state (elastic#75600)
  Fixes App Search documentation links (elastic#76133)
  Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079)
  [Resolver] Fix useSelector usage (elastic#76129)
  [Enterprise Search] Migrate util and components from ent-search (elastic#76051)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts
@fbaligand
Copy link
Contributor

Hi @spalger,

In this new implementation, is the plugin-helper able to build both old and new platform plugins?

@spalger
Copy link
Contributor Author

spalger commented Sep 3, 2020

Nope, it's only able to build KP plugins because legacy plugin support is being removed for 7.10 so there's no point in overcomplicating the tool so that it understands both. Just trying to move forward and better support the tooling with KP support.

@fbaligand
Copy link
Contributor

OK, thanks for the answer.
Given this article, I believed that legacy API support will be removed in kibana 7.11:
https://www.elastic.co/fr/blog/introducing-a-new-architecture-for-kibana

@joshdover
Copy link
Member

OK, thanks for the answer.
Given this article, I believed that legacy API support will be removed in kibana 7.11:
elastic.co/fr/blog/introducing-a-new-architecture-for-kibana

That's a mistake on my part. I'll have that post updated today. We originally estimated that this work would take longer and be a lower priority, hence the original 7.11 estimate. We decided to bump the priority of removing the legacy system for performance reasons and are now on track to remove legacy support 100% in 7.10.

@fbaligand
Copy link
Contributor

Nice to know ;)
That’s cool if it significantly impacts kibana performance!

@fastlorenzo
Copy link

OK, thanks for the answer.
Given this article, I believed that legacy API support will be removed in kibana 7.11:
elastic.co/fr/blog/introducing-a-new-architecture-for-kibana

That's a mistake on my part. I'll have that post updated today. We originally estimated that this work would take longer and be a lower priority, hence the original 7.11 estimate. We decided to bump the priority of removing the legacy system for performance reasons and are now on track to remove legacy support 100% in 7.10.

Just to confirm, is this now part of 7.10 release?

@mshustov
Copy link
Contributor

@fastlorenzo yes. The legacy plugin system has been removed in 7.10 The docs about plugin migration to Kibana platform are going be published soon #82600

@fastlorenzo
Copy link

Not sure if a bug and if related to this change, but I can no longer access the public assets from the browser since migrating to 7.10.0 (cf. #83387 )

buffer: true,
ignore: [
'**/*.d.ts',
'**/public/**',

Choose a reason for hiding this comment

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

Might be causing the public/assets not being copied to the build folder (#83387 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, that makes sense. Will get a PR up for 7.10.1 Monday.

You can probably either add the public/assets directory to the serverSourcePatterns setting.

If you're only dealing with a couple files you could also just import the file using the file-loader, ie

import mbWorkerUrl from '!!file-loader!mapbox-gl/dist/mapbox-gl-csp-worker';

Nice find, sorry about that.

Choose a reason for hiding this comment

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

Glad to hear you know how to fix it 👍

I've tried adding public/**/*, public/assets/**/*, etc in serverSourcePatterns, but nothing worked. My guess is that it's linked to the same issue.

I have a workaround for now: build the plugin with --skip-archive, manually add the content of public/assets to the build folder, and manually package the zip file.

Example with plugin named redelk:

yarn build --skip-archive
mkdir build/kibana/redelk/public
cp -R public/assets/ build/kibana/redelk/public
cd build
zip redelk-7.10.0.zip kibana -r

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks to share your workaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.10.0 v8.0.0
Projects
None yet
9 participants