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

Remove legacy plugins support #77599

Merged

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Sep 16, 2020

Summary

Fix #71927

Remove legacy plugin discovery and support from the legacy platform.

  • remove the legacy plugin system
  • unwire the legacy plugin discovery from core
  • remove the legacy xpackMain plugin
  • adapt usages and references to the legacy types

Checklist

For maintainers

Release Note

The legacy plugin system and the legacy plugin API have been removed. It is no longer possible to use third parties legacy Kibana plugins. Legacy plugin owners should migrate their plugins to the Kibana Platform plugin API

@pgayvallet pgayvallet added Feature:Legacy Removal Issues related to removing legacy Kibana release_note:breaking Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.10.0 v8.0.0 labels Sep 16, 2020
@joshdover
Copy link
Contributor

@pgayvallet I think we can also delete x-pack/legacy/plugins in this PR? Maybe even all of x-pack/legacy?

@pgayvallet
Copy link
Contributor Author

pgayvallet commented Sep 16, 2020

we can also delete x-pack/legacy/plugins in this PR? Maybe even all of x-pack/legacy?

I was going wait until #75648 and #74532 are done to do the remaining cleanup of x-pack/legacy. But yes, x-pack/index.js and all associated files will be deleted here.

@pgayvallet
Copy link
Contributor Author

Only waiting for #77703, #75648 and #74532 now

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Code & local test LGTM

Comment on lines +111 to +119
export function wrapEsError(err: any, statusCodeToMessageMap: Record<string, any> = {}) {
const { statusCode, response } = err;

const {
error: {
root_cause = [], // eslint-disable-line @typescript-eslint/naming-convention
caused_by = {}, // eslint-disable-line @typescript-eslint/naming-convention
} = {},
} = JSON.parse(response);
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the heads up!

Copy link
Contributor

@mshustov mshustov left a comment

Choose a reason for hiding this comment

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

Looks good overall. There are still a few things to remove, I'm going to add them to #56205 to do in the follow-ups.

Comment on lines -453 to +431
pluginMap: readdirSync(resolve(__dirname, 'x-pack/legacy/plugins')).reduce(
(acc, name) => {
if (!name.startsWith('_')) {
acc[name] = `x-pack/legacy/plugins/${name}`;
}
return acc;
},
{}
),
pluginMap: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't it mean that we can remove the eslint-import-resolver-kibana plugin at all?

.eslintrc.js Show resolved Hide resolved
@@ -39,8 +38,4 @@ export namespace Legacy {
export type Request = LegacyKibanaServer.Request;
export type ResponseToolkit = LegacyKibanaServer.ResponseToolkit;
export type Server = LegacyKibanaServer.Server;
Copy link
Contributor

Choose a reason for hiding this comment

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

There shouldn't be any plugin code relying on the Hapi types. Created #77997

Comment on lines 49 to 29
legacyInternals = new LegacyInternals(uiExports, config, server);
legacyInternals = new LegacyInternals();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm...IIRC it's used to read injectedVars only.

vars: vars ?? (await this.legacyInternals!.getVars('core', request)),

As we don't have any plugins to inject vars, we can delete this API. Did I miss anything?
Can be done in a follow-up. Could you create an issue, please?

src/core/server/legacy/legacy_service.ts Outdated Show resolved Hide resolved
@@ -59,7 +58,6 @@ export interface RenderingMetadata {
legacyMetadata: {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we use legacyMetadata to read uiSettings solely. Can we remove the other fields?

src/legacy/server/config/schema.js Show resolved Hide resolved
src/legacy/server/kbn_server.d.ts Outdated Show resolved Hide resolved
x-pack/dev-tools/jest/create_jest_config.js Show resolved Hide resolved
Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

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

Ingest Manager changes LGTM

Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

SIEM changes LGTM, Thank you @pgayvallet 👍

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

APM changes look good.

@@ -7,7 +7,7 @@
import { i18n } from '@kbn/i18n';
import { ANOMALY_SEVERITY } from '../../ml/common';

import { EuiTheme } from '../../../legacy/common/eui_styled_components';
import { EuiTheme } from '../../xpack_legacy/common';
Copy link
Contributor

Choose a reason for hiding this comment

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

We can actually import this from the observability plugin but it's in public there so we'll have to move that around in a separate PR.

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

Is xpack_legacy a folder you want to remove? Should we be planning to move off of that or is it fine to leave those paths like that forever?

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

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

@pgayvallet
Copy link
Contributor Author

@jasonrhodes

Is xpack_legacy a folder you want to remove? Should we be planning to move off of that or is it fine to leave those paths like that forever?

We are not planning to delete this plugin short term, even if at some point, we probably will (8.0 hopefully)

Regarding what I had to move there in this PR (the EuiThemeProvider related stuff), I created #78248 to discuss where it should actually be moved

@pgayvallet pgayvallet merged commit 0d09cea into elastic:master Sep 23, 2020
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Sep 23, 2020
* remove ALL the things.

* adapt some types and tests

* restore ensureValidConfiguration

* fix legacy service tests

* adapt uiRender mixin

* remove legacy types

* update generated doc

* restore legacy plugin schema

* update generated doc

* remove remaining code of x-pack/legacy

* adapt imports due to merge

* cleanup CODEOWNERS

* cleanup gitignore & i18nrc

* cleanup tsconfig.json

* remove unused i18n keys

* add back `"legacy/plugins/**/*",` to tsconfig until legacy space plugin is deleted

* fix create_jest_config

* remove references from eslintrc

* more eslint cleanup

* remove `x-pack/index.js`

* fix xpack gulp scripts

* fix bug with default + named imports from boom

* remove rules from eslintrc

* remove LegacyInternals

* review comments

* update generated doc

* cleanup legacy metadatas

* revert changes to eslintrc

* update generated doc
# Conflicts:
#	.github/CODEOWNERS
#	x-pack/.i18nrc.json
#	x-pack/dev-tools/jest/create_jest_config.js
#	x-pack/legacy/plugins/xpack_main/index.js
#	x-pack/legacy/server/lib/constants/index.ts
#	x-pack/legacy/server/lib/key_case_converter.js
#	x-pack/legacy/server/lib/watch_status_and_license_to_initialize.js
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
transform 500 -3 503
watcher 224 -3 227
total -6

async chunks size

id value diff baseline
infra 3.9MB -72.0B 3.9MB
ingestManager 1.1MB -36.0B 1.1MB
observability 296.5KB -36.0B 296.5KB
transform 660.4KB -520.0B 660.9KB
total -664.0B

page load bundle size

id value diff baseline
watcher 28.3KB -520.0B 28.8KB

distributable file count

id value diff baseline
default 45850 -91 45941
oss 26592 -59 26651
total -150

History

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

pgayvallet added a commit that referenced this pull request Sep 23, 2020
* remove ALL the things.

* adapt some types and tests

* restore ensureValidConfiguration

* fix legacy service tests

* adapt uiRender mixin

* remove legacy types

* update generated doc

* restore legacy plugin schema

* update generated doc

* remove remaining code of x-pack/legacy

* adapt imports due to merge

* cleanup CODEOWNERS

* cleanup gitignore & i18nrc

* cleanup tsconfig.json

* remove unused i18n keys

* add back `"legacy/plugins/**/*",` to tsconfig until legacy space plugin is deleted

* fix create_jest_config

* remove references from eslintrc

* more eslint cleanup

* remove `x-pack/index.js`

* fix xpack gulp scripts

* fix bug with default + named imports from boom

* remove rules from eslintrc

* remove LegacyInternals

* review comments

* update generated doc

* cleanup legacy metadatas

* revert changes to eslintrc

* update generated doc
# Conflicts:
#	.github/CODEOWNERS
#	x-pack/.i18nrc.json
#	x-pack/dev-tools/jest/create_jest_config.js
#	x-pack/legacy/plugins/xpack_main/index.js
#	x-pack/legacy/server/lib/constants/index.ts
#	x-pack/legacy/server/lib/key_case_converter.js
#	x-pack/legacy/server/lib/watch_status_and_license_to_initialize.js
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 23, 2020
* master: (31 commits)
  skip tests for old pacakge (elastic#78194)
  [Ingest Pipelines] Add url generator for ingest pipelines app (elastic#77872)
  [Lens] Rename "telemetry" to "stats" (elastic#78125)
  [CSM] Url search (elastic#77516)
  [Drilldowns] Config to disable URL Drilldown  (elastic#77887)
  [Lens] Combined histogram/range aggregation for numbers (elastic#76121)
  Remove legacy plugins support (elastic#77599)
  'Auto' interval must be correctly calculated for natural numbers (elastic#77995)
  [CSM] fix ingest data retry order messed up (elastic#78163)
  Add response status helpers (elastic#78006)
  Bump react-beautiful-dnd (elastic#78028)
  [Security Solution][Detection Engine] Bubbles up more error messages from ES queries to the UI (elastic#78004)
  Index pattern  - refactor constructor (elastic#77791)
  Add `xpack.security.sameSiteCookies` to docker allow list (elastic#78192)
  Remove [key: string]: any; from IIndexPattern (elastic#77968)
  Remove requirement for manage_index_templates privilege for Index Management (elastic#77377)
  [Ingest Manager] Agent bulk actions UI (elastic#77690)
  [Metrics UI] Add inventory view timeline (elastic#77804)
  Reporting/Docs: Updates for setting to enable CSV Download (elastic#78101)
  Update to latest rum-react (elastic#78193)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Legacy Removal Issues related to removing legacy Kibana release_note:breaking Team:APM All issues that need APM UI Team support Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove legacy plugin discovery