Make Core interfaces exposed to LP plugins consistent with NP plugins

[joshdover]

Summary

While working on cleaning up some type definitions, I noticed that we're not expose the same interface to the Legacy platform that will be available to New Platform plugins.

The main issue is the difference between HttpServiceSetup and CoreSetup['http']. In the internal variant, we require a pluginOpaqueId argument to some functions (eg. createRouter, registerRouteContext).

This changes that by making sure we're expose CoreSetup and CoreStart rather than the internal variants of those interfaces by building an object using a fake opaqueId that the LegacyService generates.

Risks

By using a single opaqueId for all legacy code, context providers registered by legacy code will be available to all legacy plugins. This does create a leak across plugin boundaries where legacy plugins could depend on other legacy code without realizing it.

I don't think this risk is too bad because:

1. New Platform plugins are still insulated from this problem since none of them can depend on the Legacy Platform.
2. Legacy plugins can already do this inadvertently via server.ext or via uiExports, so this doesn't quite make the problem worse, and it doesn't leak that problem to New Platform plugins.
3. It should be pretty obvious that if you're accessing context values that are not in the context.core object, you should be depending on plugins that actually provide this value.

Further improvements

We could take this one step further and solve a different problem: legacy plugins cannot currently get context registered by New Platform plugins. We could solve this by defining the fake "legacy plugin" as dependent on all other plugins in the system.

@elastic/kibana-platform what do y'all think? We will do this in a future PR.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

• This was checked for cross-browser compatibility, including a check against IE11
• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[jgowdyelastic]

ML changes LGTM

[rudolf]

We could take this one step further and solve a different problem: legacy plugins cannot currently get context registered by New Platform plugins. We could solve this by defining the fake "legacy plugin" as dependent on all other plugins in the system.

I like this idea. It's similar to how kbnServer.newPlatform.setup.plugins gives legacy plugins access to all NP plugins in a way that simulates a dependency on them.

Would this solve any immediate problems? The utility of this is somewhat limited in that it requires NP plugins which have registered context and legacy plugins who use the NP router.

[joshdover]

Would this solve any immediate problems? The utility of this is somewhat limited in that it requires NP plugins which have registered context and legacy plugins who use the NP router.

One of the things being worked on is the "search" plugin which provides different search strategies to plugins. This is a new plugin that will replace courier, and the primary way plugins would consume it would be via request and mount contexts.

Going with this option would give legacy plugins a way to start consuming these without having to move code over to NP yet.

I am going to move forward with this idea, but keep it in a separate PR.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request
• Commit: c1ca31a

[joshdover]

@restrry The integration tests are failing on this line:

https://github.com/joshdover/kibana/blob/c1ca31a6f36f39237c4564bcee9a6998e58953a0/src/core/server/http/integration_tests/core_services.test.ts#L152

          handler: kbnServer.newPlatform.setup.core.http.auth.get,

Do you know if the legacy platform actually needs this still?

[mshustov]

@joshdover there are 2 problems:

• http.auth API is not exposed to NP plugins, because we haven't finalized it yet
• typings are broken and don't warn you that platform doesn't expose this API

I sent fix in you repo joshdover#29

[joshdover]

@restrry You're too kind, thanks!

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: aba442e

[joshdover]

@elasticmachine merge upstream

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 8d976e8