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.

Sign up for GitHub

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

Jump to bottom

Remove legacy plugins support #77599

Merged
merged 37 commits into from
Sep 23, 2020
Merged

Remove legacy plugins support #77599

Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
0ac0df2
remove ALL the things.
pgayvallet Sep 16, 2020
67b4d8a
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 16, 2020
1f8249b
adapt some types and tests
pgayvallet Sep 16, 2020
4a8c362
restore ensureValidConfiguration
pgayvallet Sep 16, 2020
28db307
fix legacy service tests
pgayvallet Sep 16, 2020
5e76aeb
adapt uiRender mixin
pgayvallet Sep 16, 2020
aa4de2b
remove legacy types
pgayvallet Sep 16, 2020
f721040
update generated doc
pgayvallet Sep 16, 2020
2361249
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 16, 2020
e6fb281
restore legacy plugin schema
pgayvallet Sep 16, 2020
8467833
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 18, 2020
982494e
update generated doc
pgayvallet Sep 18, 2020
7363722
remove remaining code of x-pack/legacy
pgayvallet Sep 18, 2020
75a1a47
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 18, 2020
1498f79
adapt imports due to merge
pgayvallet Sep 18, 2020
719280a
cleanup CODEOWNERS
pgayvallet Sep 18, 2020
5e5b448
cleanup gitignore & i18nrc
pgayvallet Sep 18, 2020
1a10e3b
cleanup tsconfig.json
pgayvallet Sep 18, 2020
c5d7612
remove unused i18n keys
pgayvallet Sep 18, 2020
462388a
add back `"legacy/plugins/**/*",` to tsconfig until legacy space plug…
pgayvallet Sep 18, 2020
a20cc66
fix create_jest_config
pgayvallet Sep 18, 2020
6be4612
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 18, 2020
8443d96
remove references from eslintrc
pgayvallet Sep 18, 2020
eced562
more eslint cleanup
pgayvallet Sep 18, 2020
60e6963
remove `x-pack/index.js`
pgayvallet Sep 18, 2020
14569f0
fix xpack gulp scripts
pgayvallet Sep 18, 2020
90e5073
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 18, 2020
beed1f9
fix bug with default + named imports from boom
pgayvallet Sep 18, 2020
59572bc
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 22, 2020
6ba8682
remove rules from eslintrc
pgayvallet Sep 22, 2020
7e4a35b
remove LegacyInternals
pgayvallet Sep 22, 2020
91d2a6b
review comments
pgayvallet Sep 22, 2020
a570f53
update generated doc
pgayvallet Sep 22, 2020
ccf62c7
cleanup legacy metadatas
pgayvallet Sep 22, 2020
18bb41c
revert changes to eslintrc
pgayvallet Sep 22, 2020
ca6412c
update generated doc
pgayvallet Sep 22, 2020
7c54be0
Merge remote-tracking branch 'upstream/master' into kbn-71927-remove-…
pgayvallet Sep 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 3 additions & 46 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
* under the License.
*/

const { readdirSync } = require('fs');
const { resolve } = require('path');

const APACHE_2_0_LICENSE_HEADER = `
pgayvallet marked this conversation as resolved.
Show resolved Hide resolved
/*
* Licensed to Elasticsearch B.V. under one or more contributor
Expand Down Expand Up @@ -341,14 +338,6 @@ module.exports = {
'(src|x-pack)/plugins/**/*',
'!(src|x-pack)/plugins/**/server/**/*',

'src/legacy/core_plugins/**/*',
'!src/legacy/core_plugins/**/server/**/*',
'!src/legacy/core_plugins/**/index.{js,mjs,ts,tsx}',

'x-pack/legacy/plugins/**/*',
'!x-pack/legacy/plugins/**/server/**/*',
'!x-pack/legacy/plugins/**/index.{js,mjs,ts,tsx}',

'examples/**/*',
'!examples/**/server/**/*',
],
Expand All @@ -370,12 +359,7 @@ module.exports = {
},
{
target: ['src/core/**/*'],
from: [
'plugins/**/*',
'src/plugins/**/*',
'src/legacy/core_plugins/**/*',
'src/legacy/ui/**/*',
],
from: ['plugins/**/*', 'src/plugins/**/*', 'src/legacy/ui/**/*'],
errorMessage: 'The core cannot depend on any plugins.',
},
{
Expand All @@ -388,12 +372,6 @@ module.exports = {
target: [
'test/plugin_functional/plugins/**/public/np_ready/**/*',
'test/plugin_functional/plugins/**/server/np_ready/**/*',
'src/legacy/core_plugins/**/public/np_ready/**/*',
'src/legacy/core_plugins/vis_type_*/public/**/*',
'!src/legacy/core_plugins/vis_type_*/public/legacy*',
'src/legacy/core_plugins/**/server/np_ready/**/*',
'x-pack/legacy/plugins/**/public/np_ready/**/*',
'x-pack/legacy/plugins/**/server/np_ready/**/*',
],
allowSameFolder: true,
errorMessage:
Expand Down Expand Up @@ -450,15 +428,7 @@ module.exports = {
forceNode: false,
rootPackageName: 'kibana',
kibanaPath: '.',
pluginMap: readdirSync(resolve(__dirname, 'x-pack/legacy/plugins')).reduce(
(acc, name) => {
if (!name.startsWith('_')) {
acc[name] = `x-pack/legacy/plugins/${name}`;
}
return acc;
},
{}
),
pluginMap: {},
Comment on lines -453 to +428
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure this rule makes any sense without any legacy plugin. I changed pluginMap to {}, but should I just remove the whole rule instead @elastic/kibana-operations ?

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 that the custom eslint-import-resolver-kibana settings for x-pack are only required for the legacy import syntax support for things like ui/* and plugins/*. I suspect that x-pack can use the root settings and we can get rid of these settings.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually still necessary. See https://github.com/elastic/kibana/pull/77599/checks?check_run_id=1147976384

},
},
},
Expand Down Expand Up @@ -764,16 +734,6 @@ module.exports = {
},
},

/**
* GIS overrides
*/
{
files: ['x-pack/legacy/plugins/maps/**/*.js'],
rules: {
'react/prefer-stateless-function': [0, { ignorePureComponents: false }],
},
},

/**
* ML overrides
*/
Expand Down Expand Up @@ -1238,10 +1198,7 @@ module.exports = {
* TSVB overrides
*/
{
files: [
'src/plugins/vis_type_timeseries/**/*.{js,mjs,ts,tsx}',
'src/legacy/core_plugins/vis_type_timeseries/**/*.{js,mjs,ts,tsx}',
],
files: ['src/plugins/vis_type_timeseries/**/*.{js,mjs,ts,tsx}'],
rules: {
'import/no-default-export': 'error',
},
Expand Down
13 changes: 2 additions & 11 deletions .github/CODEOWNERS
View file
Open in desktop
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
/x-pack/plugins/apm/server/projections/rum_overview.ts @elastic/uptime

# Beats
/x-pack/legacy/plugins/beats_management/ @elastic/beats
/x-pack/plugins/beats_management/ @elastic/beats

# Canvas
/x-pack/plugins/canvas/ @elastic/kibana-canvas
Expand All @@ -86,16 +86,13 @@
/x-pack/plugins/global_search_bar/ @elastic/kibana-core-ui

# Observability UIs
/x-pack/legacy/plugins/infra/ @elastic/logs-metrics-ui
/x-pack/plugins/infra/ @elastic/logs-metrics-ui
/x-pack/plugins/ingest_manager/ @elastic/ingest-management
/x-pack/legacy/plugins/ingest_manager/ @elastic/ingest-management
/x-pack/plugins/observability/ @elastic/observability-ui
/x-pack/plugins/monitoring/ @elastic/stack-monitoring-ui
/x-pack/plugins/uptime @elastic/uptime

# Machine Learning
/x-pack/legacy/plugins/ml/ @elastic/ml-ui
/x-pack/plugins/ml/ @elastic/ml-ui
/x-pack/test/functional/apps/machine_learning/ @elastic/ml-ui
/x-pack/test/functional/services/machine_learning/ @elastic/ml-ui
Expand All @@ -107,7 +104,6 @@
/x-pack/test/functional/services/transform.ts @elastic/ml-ui

# Maps
/x-pack/legacy/plugins/maps/ @elastic/kibana-gis
/x-pack/plugins/maps/ @elastic/kibana-gis
/x-pack/test/api_integration/apis/maps/ @elastic/kibana-gis
/x-pack/test/functional/apps/maps/ @elastic/kibana-gis
Expand Down Expand Up @@ -234,13 +230,8 @@ x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json @elastic/kib
/src/plugins/dev_tools/ @elastic/es-ui
/src/plugins/console/ @elastic/es-ui
/src/plugins/es_ui_shared/ @elastic/es-ui
/x-pack/legacy/plugins/cross_cluster_replication/ @elastic/es-ui
/x-pack/plugins/cross_cluster_replication/ @elastic/es-ui
/x-pack/plugins/index_lifecycle_management/ @elastic/es-ui
/x-pack/legacy/plugins/index_management/ @elastic/es-ui
/x-pack/legacy/plugins/license_management/ @elastic/es-ui
/x-pack/legacy/plugins/rollup/ @elastic/es-ui
/x-pack/legacy/plugins/snapshot_restore/ @elastic/es-ui
/x-pack/legacy/plugins/upgrade_assistant/ @elastic/es-ui
/x-pack/plugins/console_extensions/ @elastic/es-ui
/x-pack/plugins/es_ui_shared/ @elastic/es-ui
/x-pack/plugins/grokdebugger/ @elastic/es-ui
Expand Down
5 changes: 0 additions & 5 deletions kibana.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export { Public, Server };
/**
* All exports from TS ambient definitions (where types are added for JS source in a .d.ts file).
*/
import * as LegacyKibanaPluginSpec from './src/legacy/plugin_discovery/plugin_spec/plugin_spec_options';
import * as LegacyKibanaServer from './src/legacy/server/kbn_server';

/**
Expand All @@ -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


export type InitPluginFunction = LegacyKibanaPluginSpec.InitPluginFunction;
export type UiExports = LegacyKibanaPluginSpec.UiExports;
export type PluginSpecOptions = LegacyKibanaPluginSpec.PluginSpecOptions;
}
16 changes: 1 addition & 15 deletions src/cli/serve/serve.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ const CAN_CLUSTER = canRequire(CLUSTER_MANAGER_PATH);
const REPL_PATH = resolve(__dirname, '../repl');
const CAN_REPL = canRequire(REPL_PATH);

// xpack is installed in both dev and the distributable, it's optional if
// install is a link to the source, not an actual install
const XPACK_DIR = resolve(__dirname, '../../../x-pack');
const XPACK_INSTALLED = canRequire(XPACK_DIR);

const pathCollector = function () {
const paths = [];
return function (path) {
Expand Down Expand Up @@ -137,16 +132,7 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) {
if (opts.logFile) set('logging.dest', opts.logFile);

set('plugins.scanDirs', _.compact([].concat(get('plugins.scanDirs'), opts.pluginDir)));
set(
'plugins.paths',
_.compact(
[].concat(
get('plugins.paths'),
opts.pluginPath,
XPACK_INSTALLED && !opts.oss ? [XPACK_DIR] : []
)
)
);
set('plugins.paths', _.compact([].concat(get('plugins.paths'), opts.pluginPath)));
Comment on lines -140 to +135
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was only used to specify the legacy xpack plugin's path. KP's xpack path is handled in packages/kbn-config/src/env.ts


merge(extraCliOptions);
merge(readKeystore());
Expand Down
2 changes: 0 additions & 2 deletions src/core/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,7 @@ export {
export {
LegacyServiceSetupDeps,
LegacyServiceStartDeps,
LegacyServiceDiscoverPlugins,
LegacyConfig,
LegacyUiExports,
LegacyInternals,
} from './legacy';

Expand Down
7 changes: 1 addition & 6 deletions src/core/server/legacy/config/ensure_valid_configuration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,12 @@ describe('ensureValidConfiguration', () => {
configService as any,
{
settings: 'settings',
pluginSpecs: 'pluginSpecs',
disabledPluginSpecs: 'disabledPluginSpecs',
pluginExtendedConfig: 'pluginExtendedConfig',
uiExports: 'uiExports',
legacyConfig: 'pluginExtendedConfig',
} as any
);
expect(getUnusedConfigKeys).toHaveBeenCalledTimes(1);
expect(getUnusedConfigKeys).toHaveBeenCalledWith({
coreHandledConfigPaths: ['core', 'elastic'],
pluginSpecs: 'pluginSpecs',
disabledPluginSpecs: 'disabledPluginSpecs',
settings: 'settings',
legacyConfig: 'pluginExtendedConfig',
});
Expand Down
8 changes: 3 additions & 5 deletions src/core/server/legacy/config/ensure_valid_configuration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@

import { getUnusedConfigKeys } from './get_unused_config_keys';
import { ConfigService } from '../../config';
import { LegacyServiceDiscoverPlugins } from '../types';
import { CriticalError } from '../../errors';
import { LegacyServiceSetupConfig } from '../types';

export async function ensureValidConfiguration(
configService: ConfigService,
{ pluginSpecs, disabledPluginSpecs, pluginExtendedConfig, settings }: LegacyServiceDiscoverPlugins
{ legacyConfig, settings }: LegacyServiceSetupConfig
) {
const unusedConfigKeys = await getUnusedConfigKeys({
coreHandledConfigPaths: await configService.getUsedPaths(),
pluginSpecs,
disabledPluginSpecs,
settings,
legacyConfig: pluginExtendedConfig,
legacyConfig,
});

if (unusedConfigKeys.length > 0) {
Expand Down
48 changes: 1 addition & 47 deletions src/core/server/legacy/config/get_unused_config_keys.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { LegacyPluginSpec, LegacyConfig, LegacyVars } from '../types';
import { LegacyConfig, LegacyVars } from '../types';
import { getUnusedConfigKeys } from './get_unused_config_keys';

describe('getUnusedConfigKeys', () => {
Expand All @@ -35,8 +35,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {},
legacyConfig: getConfig(),
})
Expand All @@ -47,8 +45,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
presentInBoth: true,
alsoInBoth: 'someValue',
Expand All @@ -65,8 +61,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
presentInBoth: true,
},
Expand All @@ -82,8 +76,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
presentInBoth: true,
onlyInSetting: 'value',
Expand All @@ -99,8 +91,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
elasticsearch: {
username: 'foo',
Expand All @@ -121,8 +111,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
env: 'development',
},
Expand All @@ -139,8 +127,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
prop: ['a', 'b', 'c'],
},
Expand All @@ -152,40 +138,10 @@ describe('getUnusedConfigKeys', () => {
});
});

it('ignores config for plugins that are disabled', async () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: [],
pluginSpecs: [],
disabledPluginSpecs: [
({
id: 'foo',
getConfigPrefix: () => 'foo.bar',
} as unknown) as LegacyPluginSpec,
],
settings: {
foo: {
bar: {
unused: true,
},
},
plugin: {
missingProp: false,
},
},
legacyConfig: getConfig({
prop: 'a',
}),
})
).toEqual(['plugin.missingProp']);
});

it('ignores properties managed by the new platform', async () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: ['core', 'foo.bar'],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
core: {
prop: 'value',
Expand All @@ -204,8 +160,6 @@ describe('getUnusedConfigKeys', () => {
expect(
await getUnusedConfigKeys({
coreHandledConfigPaths: ['core', 'array'],
pluginSpecs: [],
disabledPluginSpecs: [],
settings: {
core: {
prop: 'value',
Expand Down
12 changes: 1 addition & 11 deletions src/core/server/legacy/config/get_unused_config_keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,20 @@

import { difference } from 'lodash';
import { getFlattenedObject } from '@kbn/std';
import { unset } from '../../../../legacy/utils';
import { hasConfigPathIntersection } from '../../config';
import { LegacyPluginSpec, LegacyConfig, LegacyVars } from '../types';
import { LegacyConfig, LegacyVars } from '../types';

const getFlattenedKeys = (object: object) => Object.keys(getFlattenedObject(object));

export async function getUnusedConfigKeys({
coreHandledConfigPaths,
pluginSpecs,
disabledPluginSpecs,
settings,
legacyConfig,
}: {
coreHandledConfigPaths: string[];
pluginSpecs: LegacyPluginSpec[];
disabledPluginSpecs: LegacyPluginSpec[];
settings: LegacyVars;
legacyConfig: LegacyConfig;
}) {
// remove config values from disabled plugins
for (const spec of disabledPluginSpecs) {
unset(settings, spec.getConfigPrefix());
}

const inputKeys = getFlattenedKeys(settings);
const appliedKeys = getFlattenedKeys(legacyConfig.get());

Expand Down
Loading