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 plugin discovery #71927

Closed
14 tasks done
joshdover opened this issue Jul 15, 2020 · 20 comments · Fixed by #77599
Closed
14 tasks done

Remove legacy plugin discovery #71927

joshdover opened this issue Jul 15, 2020 · 20 comments · Fixed by #77599
Assignees
Labels
Feature:New Platform release_note:breaking ReleaseStatus Item of high enough importance that it should be called out in release status meetings Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@joshdover
Copy link
Contributor

joshdover commented Jul 15, 2020

@joshdover joshdover added Feature:New Platform release_note:breaking Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Jul 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet
Copy link
Contributor

pgayvallet commented Jul 23, 2020

We are very close to being able to remove the legacy optimizer, however we still got some work to do before we will be able to totally drop legacy plugin discovery, as there are some remaining.

oss

apm-oss

Done in #73016

console_legacy

This is used to expose elasticsearchUrl to the ui

async init(server: any) {
_legacyEsConfig = await server.newPlatform.__internals.elasticsearch.legacy.config$
.pipe(first())
.toPromise();
},
uiExports: {
injectDefaultVars: () => ({
elasticsearchUrl: url.format(
Object.assign(url.parse(head(_legacyEsConfig.hosts) as any), { auth: false })
),
}),

I thought it could be done by exposing an endpoint that would access elasticsearch.legacy.config$, but I was wrong, as this property is only exposed on the internal / legacy contract. I moved #68497 back to 7.10, this removal is blocked by it.

cc @elastic/es-ui

elasticsearch

It's exposing things like createCluster and getCluster, which may be removed if no more legacy plugins are using them (did not check usages in the legacy server code itself), but it's also exposing waitUntilReady which is tightly coupled with the legacy server itself. I guess this plugin will have to stay until the end of legacy.

EDIT: it seems it is also registering proxy routes to ES

server.route({
method: 'POST',
path: '/elasticsearch/_msearch',
config: {
payload: {
parse: 'gunzip',
},
},

Not sure who is consuming these, does anyone have an idea?

kibana

Still doing a few things:

uiExports: {
styleSheetPaths: resolve(__dirname, 'public/index.scss'),
uiSettingDefaults: getUiSettingDefaults(),
},

Not sure if related to #22779 or #59755

  • Registering the csp usage collector

init: async function (server) {
const { usageCollection } = server.newPlatform.setup.plugins;
registerCspCollector(usageCollection, server);
},

Not sure I saw this one listed anywhere. csp config is now in core, so I guess this should probably be moved there, but we need a way to access usageCollection plugin from within core. Seems related to #56762 (comment)

  • create the data folder

created #73002

status_page

Done in #72017.

The empty app plugin left should safely be removed once the legacy optimizer is no more.

tests_bundle

I have no idea what this is for. I see some karma references, is this only used for karma testing @elastic/kibana-operations?

x-pack

beats_management

Covered by #70930

Monitoring

Used to bridge the legacy status API to the KP plugin

init(server: Hapi.Server) {
const npMonitoring = server.newPlatform.setup.plugins.monitoring as object & {
registerLegacyAPI: (api: unknown) => void;
};
if (npMonitoring) {
const kbnServerStatus = this.kbnServer.status;
npMonitoring.registerLegacyAPI({
getServerStatus: () => {
const status = kbnServerStatus.toJSON();
return status?.overall?.state;
},
});
}
},

Depends on #41983

security

Only here to expose some hacks (x-pack/legacy/plugins/security/public/hacks/legacy.ts) for the legacy apps. Should be good to remove as we no longer have any legacy apps? Or is this still used for management tabs @elastic/kibana-security

spaces

  • Used to register some redirection routes

import routes from 'ui/routes';
import { SpacesPluginSetup } from '../../../../plugins/spaces/public';
const spaces = (npSetup.plugins as any).spaces as SpacesPluginSetup;
if (spaces) {
routes.when('/management/spaces/list', { redirectTo: '/management/kibana/spaces' });
routes.when('/management/spaces/create', { redirectTo: '/management/kibana/spaces/create' });
routes.when('/management/spaces/edit/:spaceId', {
redirectTo: '/management/kibana/spaces/edit/:spaceId',
});
}

Not sure if these are still really used now that we don't have any app running in legacy mode? @elastic/kibana-security

  • expose the replaceInjectedVars in uiExports.

Seems to be used to add the activeSpace var. Not sure if this is still relevant @elastic/kibana-security?

  • expose things such as getSpaceId and getActiveSpace on the server

This seems fine to remove once we no longer have any other xpack server-side legacy plugin.

xpack_main

This one is doing quite a few things. I guess this will be the last xpack plugin to be removed.

@sorenlouv
Copy link
Member

sorenlouv commented Jul 23, 2020

@elastic/apm-ui Is this still used anywhere, or can the legacy apm_oss plugin be removed?

apm_oss is a dependency of xpack.apm so cannot be removed now or in the future. However, looking closer I see there are two apm_oss plugins:

I think that legacy/core_plugins/apm_oss can be merged into plugins/apm_oss.

From what I see, the remains are only used to expose indexPatterns to the server

I'm not sure if server.expose is still used for anything. it was added by @tylersmalley in #29845 so maybe he knows.

@pgayvallet
Copy link
Contributor

apm_oss is a dependency of xpack.apm so cannot be removed now or in the future. However, looking closer I see there are two apm_oss plugins:

Yea, we are only talking about legacy removal here (src/legacy/core_plugins/apm_oss)

@mshustov
Copy link
Contributor

Exporting some uiSettingDefaults

That's #59755 and #63459 not sure it's a real blocker. we can register them manually

@azasypkin
Copy link
Member

Security
Only here to expose some hacks (x-pack/legacy/plugins/security/public/hacks/legacy.ts) for the legacy apps. Should be good to remove as we no longer have any legacy apps? Or is this still used for management tabs @elastic/kibana-security

Once we don't have any Angular based plugins then we can get rid of this completely. Maintaining /account ---> /security/account redirect isn't critical.

Spaces
Used to register some redirection routes
Not sure if these are still really used now that we don't have any app running in legacy mode? @elastic/kibana-security

Since these redirects don't seem to work right now anyway (likely because Management plugin migrated away from ui/routes, I think it's reasonable to drop legacy links (@legrego can keep me honest here).

expose the replaceInjectedVars in uiExports.
Seems to be used to add the activeSpace var. Not sure if this is still relevant @elastic/kibana-security?

It seems Infra is still using this. I believe they can switch to the contract Spaces KP plugins exposes now.

expose things such as getSpaceId and getActiveSpace on the server
This seems fine to remove once we no longer have any other xpack server-side legacy plugin.

Sounds good to me.

@pgayvallet
Copy link
Contributor

It seems Infra is still using this. I believe they can switch to the contract Spaces KP plugins exposes now.

@elastic/logs-metrics-ui do you have an issue to replace these space injectedVars with the spaces plugin API?

@tylersmalley
Copy link
Contributor

tests_bundle

I have no idea what this is for. I see some karma references, is this only used for karma testing @elastic/kibana-operations?

That's my understanding. @spalger is there anything else this is used for? Can we remove once all Karma test are removed (related: #71002)?

@spalger
Copy link
Contributor

spalger commented Jul 23, 2020

Yeah, this will go along with karma and the legacy optimizer

@cjcenizal
Copy link
Contributor

elasticsearch

@pgayvallet It looks like https://github.com/elastic/kibana/blob/master/src/plugins/home/public/application/components/tutorial/tutorial.js#L206 makes a request to /elasticsearch/_msearch.

@weltenwort
Copy link
Member

@elastic/logs-metrics-ui do you have an issue to replace these space injectedVars with the spaces plugin API?

Yes, we track it in #68142

@joshdover
Copy link
Contributor Author

Not sure I saw this one listed anywhere. csp config is now in core, so I guess this should probably be moved there, but we need a way to access usageCollection plugin from within core. Seems related to #56762 (comment)

@elastic/kibana-telemetry any problems with us moving the CSP usage collector directly to the telemetry plugin for the time being?

@joshdover
Copy link
Contributor Author

EDIT: it seems it is also registering proxy routes to ES

server.route({
method: 'POST',
path: '/elasticsearch/_msearch',
config: {
payload: {
parse: 'gunzip',
},
},

Not sure who is consuming these, does anyone have an idea?

I have opened new issues for each usage of these:
#73992
#73993

@flash1293
Copy link
Contributor

@joshdover seems like the issue in the kibana app section is actually on core UI, can you confirm?

@afharo
Copy link
Member

afharo commented Aug 4, 2020

Not sure I saw this one listed anywhere. csp config is now in core, so I guess this should probably be moved there, but we need a way to access usageCollection plugin from within core. Seems related to #56762 (comment)

@elastic/kibana-telemetry any problems with us moving the CSP usage collector directly to the telemetry plugin for the time being?

@joshdover after reviewing the collector, it looks like it only accesses to core public values. No problem on my end to move it to the core-but-outside-core collectors in the plugin kibana_usage_collection. Or any other plugin owned by the platform team :)

@legrego
Copy link
Member

legrego commented Aug 4, 2020

Not sure I saw this one listed anywhere. csp config is now in core, so I guess this should probably be moved there, but we need a way to access usageCollection plugin from within core. Seems related to #56762 (comment)

@elastic/kibana-telemetry any problems with us moving the CSP usage collector directly to the telemetry plugin for the time being?

No objections here either. I wasn't aware that CSP even had a usage collector :)

@weltenwort
Copy link
Member

@elastic/logs-metrics-ui do you have an issue to replace these space injectedVars with the spaces plugin API?

I removed the getInjectedVars() usage from the infra plugin in #74280, which closes #68142. Is there anything else in the plugin that blocks you in this effort?

@joshdover
Copy link
Contributor Author

@weltenwort Not that I am aware of, thanks Felix

@pgayvallet
Copy link
Contributor

Added #76416 to the list of blockers.

@pgayvallet pgayvallet self-assigned this Sep 16, 2020
@stacey-gammon stacey-gammon added the ReleaseStatus Item of high enough importance that it should be called out in release status meetings label Sep 17, 2020
@pgayvallet
Copy link
Contributor

All blockers have been addressed. #77599 is the final PR that is going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:breaking ReleaseStatus Item of high enough importance that it should be called out in release status meetings Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.