From c1ab1694684d42d71d9ff168e93c37c36e3bcdee Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Tue, 1 Oct 2019 14:20:47 -0500 Subject: [PATCH] Make core interfaces exposed to legacy platform consistent --- src/core/public/core_system.test.ts | 1 + src/core/public/core_system.ts | 16 +++++--- src/core/public/legacy/legacy_service.ts | 2 + src/core/server/legacy/legacy_service.test.ts | 23 +++--------- src/core/server/legacy/legacy_service.ts | 37 +++++++++++++++++-- src/core/server/server.test.ts | 1 + src/core/server/server.ts | 6 ++- src/legacy/server/index_patterns/routes.ts | 15 +++----- src/legacy/server/kbn_server.d.ts | 13 +++++-- .../plugins/core_plugin_legacy/index.ts | 2 +- x-pack/legacy/plugins/lens/index.ts | 3 +- .../plugins/lens/server/routes/field_stats.ts | 4 +- .../plugins/lens/server/routes/index_stats.ts | 3 +- .../plugins/ml/server/new_platform/plugin.ts | 5 ++- 14 files changed, 82 insertions(+), 49 deletions(-) diff --git a/src/core/public/core_system.test.ts b/src/core/public/core_system.test.ts index 36a1393501ea3d6..460f2b3839aec55 100644 --- a/src/core/public/core_system.test.ts +++ b/src/core/public/core_system.test.ts @@ -62,6 +62,7 @@ const defaultCoreSystemParams = { beforeEach(() => { jest.clearAllMocks(); + MockPluginsService.getOpaqueIds.mockReturnValue(new Map()); }); function createCoreSystem(params = {}) { diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 7b9ed50f0959101..732b8384d50abdf 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -80,7 +80,7 @@ export interface InternalCoreStart extends Omit { export class CoreSystem { private readonly fatalErrors: FatalErrorsService; private readonly injectedMetadata: InjectedMetadataService; - private readonly legacyPlatform: LegacyPlatformService; + private readonly legacy: LegacyPlatformService; private readonly notifications: NotificationsService; private readonly http: HttpService; private readonly savedObjects: SavedObjectsService; @@ -134,7 +134,7 @@ export class CoreSystem { this.context = new ContextService(this.coreContext); this.plugins = new PluginsService(this.coreContext, injectedMetadata.uiPlugins); - this.legacyPlatform = new LegacyPlatformService({ + this.legacy = new LegacyPlatformService({ requireLegacyFiles, useLegacyTestHarness, }); @@ -154,7 +154,11 @@ export class CoreSystem { const notifications = this.notifications.setup({ uiSettings }); const pluginDependencies = this.plugins.getOpaqueIds(); - const context = this.context.setup({ pluginDependencies }); + const context = this.context.setup({ + // We inject a fake "legacy plugin" with no dependencies so that legacy plugins can register context providers + // that will only be available to other legacy plugins and will not leak into New Platform plugins. + pluginDependencies: new Map([...pluginDependencies, [this.legacy.legacyId, []]]), + }); const application = this.application.setup({ context }); const core: InternalCoreSetup = { @@ -170,7 +174,7 @@ export class CoreSystem { // Services that do not expose contracts at setup const plugins = await this.plugins.setup(core); - await this.legacyPlatform.setup({ + await this.legacy.setup({ core, plugins: mapToObject(plugins.contracts), }); @@ -260,7 +264,7 @@ export class CoreSystem { targetDomElement: coreUiTargetDomElement, }); - await this.legacyPlatform.start({ + await this.legacy.start({ core, plugins: mapToObject(plugins.contracts), targetDomElement: rendering.legacyTargetDomElement, @@ -277,7 +281,7 @@ export class CoreSystem { } public stop() { - this.legacyPlatform.stop(); + this.legacy.stop(); this.plugins.stop(); this.notifications.stop(); this.http.stop(); diff --git a/src/core/public/legacy/legacy_service.ts b/src/core/public/legacy/legacy_service.ts index ba93cd7b6b5a71a..79fa32040b14ca7 100644 --- a/src/core/public/legacy/legacy_service.ts +++ b/src/core/public/legacy/legacy_service.ts @@ -51,6 +51,8 @@ interface BootstrapModule { * setup either the app or browser tests. */ export class LegacyPlatformService { + /** Symbol to represent the legacy platform as a fake "plugin". Used by the ContextService */ + public readonly legacyId = Symbol(); private bootstrapModule?: BootstrapModule; private targetDomElement?: HTMLElement; diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index cf72bb72079e9c4..b5153847e2d0fa3 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -46,6 +46,7 @@ import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins'; import { PluginsServiceSetup, PluginsServiceStart } from '../plugins/plugins_service'; import { SavedObjectsServiceStart } from 'src/core/server/saved_objects/saved_objects_service'; import { KibanaMigrator } from '../saved_objects/migrations'; +import { httpServiceMock } from '../http/http_service.mock'; const MockKbnServer: jest.Mock = KbnServer as any; @@ -86,6 +87,7 @@ beforeEach(() => { context: contextServiceMock.createSetupContract(), elasticsearch: { legacy: {} } as any, http: { + ...httpServiceMock.createSetupContract(), auth: { getAuthHeaders: () => undefined, }, @@ -146,12 +148,7 @@ describe('once LegacyService is set up with connection info', () => { expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { server: { autoListen: true } }, - { - setupDeps, - startDeps, - handledConfigPaths: ['foo.bar'], - logger, - }, + expect.any(Object), { disabledPluginSpecs: [], pluginSpecs: [], uiExports: [] } ); @@ -176,12 +173,7 @@ describe('once LegacyService is set up with connection info', () => { expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { server: { autoListen: true } }, - { - setupDeps, - startDeps, - handledConfigPaths: ['foo.bar'], - logger, - }, + expect.any(Object), { disabledPluginSpecs: [], pluginSpecs: [], uiExports: [] } ); @@ -311,12 +303,7 @@ describe('once LegacyService is set up without connection info', () => { expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { server: { autoListen: true } }, - { - setupDeps, - startDeps, - handledConfigPaths: ['foo.bar'], - logger, - }, + expect.any(Object), { disabledPluginSpecs: [], pluginSpecs: [], uiExports: [] } ); }); diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 268e5f553723fff..a944f259d9a2dff 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -20,7 +20,7 @@ import { combineLatest, ConnectableObservable, EMPTY, Observable, Subscription } from 'rxjs'; import { first, map, publishReplay, tap } from 'rxjs/operators'; import { CoreService } from '../../types'; -import { InternalCoreSetup, InternalCoreStart } from '../'; +import { InternalCoreSetup, InternalCoreStart, CoreSetup, CoreStart } from '../'; import { SavedObjectsLegacyUiExports } from '../types'; import { Config } from '../config'; import { CoreContext } from '../core_context'; @@ -81,6 +81,8 @@ export interface LegacyServiceSetup { /** @internal */ export class LegacyService implements CoreService { + /** Symbol to represent the legacy platform as a fake "plugin". Used by the ContextService */ + public readonly legacyId = Symbol(); private readonly log: Logger; private readonly devConfig$: Observable; private readonly httpConfig$: Observable; @@ -226,6 +228,29 @@ export class LegacyService implements CoreService { uiExports: SavedObjectsLegacyUiExports; } ) { + const coreSetup: CoreSetup = { + context: setupDeps.core.context, + elasticsearch: { + adminClient$: setupDeps.core.elasticsearch.adminClient$, + dataClient$: setupDeps.core.elasticsearch.dataClient$, + createClient: setupDeps.core.elasticsearch.createClient, + }, + http: { + createCookieSessionStorageFactory: setupDeps.core.http.createCookieSessionStorageFactory, + registerRouteHandlerContext: setupDeps.core.http.registerRouteHandlerContext.bind( + null, + this.legacyId + ), + createRouter: () => setupDeps.core.http.createRouter('', this.legacyId), + registerOnPreAuth: setupDeps.core.http.registerOnPreAuth, + registerAuth: setupDeps.core.http.registerAuth, + registerOnPostAuth: setupDeps.core.http.registerOnPostAuth, + basePath: setupDeps.core.http.basePath, + isTlsEnabled: setupDeps.core.http.isTlsEnabled, + }, + }; + const coreStart: CoreStart = {}; + // eslint-disable-next-line @typescript-eslint/no-var-requires const KbnServer = require('../../../legacy/server/kbn_server'); const kbnServer: LegacyKbnServer = new KbnServer( @@ -233,8 +258,14 @@ export class LegacyService implements CoreService { config, { handledConfigPaths: await this.coreContext.configService.getUsedPaths(), - setupDeps, - startDeps, + setupDeps: { + core: coreSetup, + plugins: setupDeps.plugins, + }, + startDeps: { + core: coreStart, + plugins: startDeps.plugins, + }, logger: this.coreContext.logger, }, legacyPlugins diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index cb1a88f6e8aed87..7319e4126fc07be 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -38,6 +38,7 @@ const logger = loggingServiceMock.create(); beforeEach(() => { mockConfigService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); + mockPluginsService.discover.mockResolvedValue(new Map()); }); afterEach(() => { diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 2b63d6ac3be1c93..29c9a3cd09275bf 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -72,7 +72,11 @@ export class Server { // Discover any plugins before continuing. This allows other systems to utilize the plugin dependency graph. const pluginDependencies = await this.plugins.discover(); - const contextServiceSetup = this.context.setup({ pluginDependencies }); + const contextServiceSetup = this.context.setup({ + // We inject a fake "legacy plugin" with no dependencies so that legacy plugins can register context providers + // that will only be available to other legacy plugins and will not leak into New Platform plugins. + pluginDependencies: new Map([...pluginDependencies, [this.legacy.legacyId, []]]), + }); const httpSetup = await this.http.setup({ context: contextServiceSetup, diff --git a/src/legacy/server/index_patterns/routes.ts b/src/legacy/server/index_patterns/routes.ts index 4fceae48fc5876e..60b2ea28afab504 100644 --- a/src/legacy/server/index_patterns/routes.ts +++ b/src/legacy/server/index_patterns/routes.ts @@ -19,15 +19,10 @@ import { first } from 'rxjs/operators'; import { schema } from '@kbn/config-schema'; -import { - InternalCoreSetup, - KibanaRequest, - RequestHandlerContext, - APICaller, -} from '../../../core/server'; +import { CoreSetup, KibanaRequest, RequestHandlerContext, APICaller } from '../../../core/server'; import { IndexPatternsService } from './service'; -export function registerRoutes(core: InternalCoreSetup) { +export function registerRoutes(core: CoreSetup) { const getIndexPatternsService = async (request: KibanaRequest): Promise => { const client = await core.elasticsearch.dataClient$.pipe(first()).toPromise(); const callCluster: APICaller = (endpoint, params, options) => @@ -45,10 +40,10 @@ export function registerRoutes(core: InternalCoreSetup) { return parsedFields; }; - const router = core.http.createRouter('/api/index_patterns'); + const router = core.http.createRouter(); router.get( { - path: '/_fields_for_wildcard', + path: '/api/index_patterns/_fields_for_wildcard', validate: { query: schema.object({ pattern: schema.string(), @@ -89,7 +84,7 @@ export function registerRoutes(core: InternalCoreSetup) { router.get( { - path: '/_fields_for_time_pattern', + path: '/api/index_patterns/_fields_for_time_pattern', validate: { query: schema.object({ pattern: schema.string(), diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 406697ab65d8f60..5407d20c905af7d 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -20,7 +20,7 @@ import { ResponseObject, Server } from 'hapi'; import { UnwrapPromise } from '@kbn/utility-types'; -import { SavedObjectsClientProviderOptions } from 'src/core/server'; +import { SavedObjectsClientProviderOptions, CoreSetup } from 'src/core/server'; import { ConfigService, ElasticsearchServiceSetup, @@ -78,6 +78,7 @@ declare module 'hapi' { factoryFn: (request: Request) => Record ) => void; uiSettingsServiceFactory: (options: any) => any; + newPlatform: KbnServer['newPlatform']; } interface Request { @@ -100,8 +101,14 @@ export default class KbnServer { coreContext: { logger: LoggerFactory; }; - setup: LegacyServiceSetupDeps; - start: LegacyServiceStartDeps; + setup: { + core: CoreSetup; + plugins: Record; + }; + start: { + core: CoreSetup; + plugins: Record; + }; stop: null; params: { handledConfigPaths: UnwrapPromise>; diff --git a/test/plugin_functional/plugins/core_plugin_legacy/index.ts b/test/plugin_functional/plugins/core_plugin_legacy/index.ts index b0536106dcf6c9b..b9cd4a7e8bb34cc 100644 --- a/test/plugin_functional/plugins/core_plugin_legacy/index.ts +++ b/test/plugin_functional/plugins/core_plugin_legacy/index.ts @@ -26,7 +26,7 @@ export default function(kibana: any) { require: ['kibana'], init(server: KbnServer) { const { http } = server.newPlatform.setup.core; - const router = http.createRouter(''); + const router = http.createRouter(); router.get({ path: '/api/np-http-in-legacy', validate: false }, async (context, req, res) => { const response = await context.core.elasticsearch.adminClient.callAsInternalUser('ping'); diff --git a/x-pack/legacy/plugins/lens/index.ts b/x-pack/legacy/plugins/lens/index.ts index 399a65041b66466..0e25f2e11ad62a5 100644 --- a/x-pack/legacy/plugins/lens/index.ts +++ b/x-pack/legacy/plugins/lens/index.ts @@ -10,7 +10,7 @@ import { LegacyPluginInitializer } from 'src/legacy/types'; import KbnServer, { Server } from 'src/legacy/server/kbn_server'; import { CoreSetup } from 'src/core/server'; import mappings from './mappings.json'; -import { PLUGIN_ID, getEditPath, BASE_API_URL } from './common'; +import { PLUGIN_ID, getEditPath } from './common'; import { lensServerPlugin } from './server'; const NOT_INTERNATIONALIZED_PRODUCT_NAME = 'Lens Visualizations'; @@ -87,7 +87,6 @@ export const lens: LegacyPluginInitializer = kibana => { plugin.setup(({ http: { ...kbnServer.newPlatform.setup.core.http, - createRouter: () => kbnServer.newPlatform.setup.core.http.createRouter(BASE_API_URL), }, } as unknown) as CoreSetup); diff --git a/x-pack/legacy/plugins/lens/server/routes/field_stats.ts b/x-pack/legacy/plugins/lens/server/routes/field_stats.ts index a57811362c6cf3f..c53a78a63c46b32 100644 --- a/x-pack/legacy/plugins/lens/server/routes/field_stats.ts +++ b/x-pack/legacy/plugins/lens/server/routes/field_stats.ts @@ -9,7 +9,7 @@ import DateMath from '@elastic/datemath'; import { schema } from '@kbn/config-schema'; import { AggregationSearchResponse } from 'elasticsearch'; import { CoreSetup } from 'src/core/server'; -import { FieldStatsResponse } from '../../common'; +import { FieldStatsResponse, BASE_API_URL } from '../../common'; const SHARD_SIZE = 5000; @@ -17,7 +17,7 @@ export async function initFieldsRoute(setup: CoreSetup) { const router = setup.http.createRouter(); router.post( { - path: '/index_stats/{indexPatternTitle}/field', + path: `${BASE_API_URL}/index_stats/{indexPatternTitle}/field`, validate: { params: schema.object({ indexPatternTitle: schema.string(), diff --git a/x-pack/legacy/plugins/lens/server/routes/index_stats.ts b/x-pack/legacy/plugins/lens/server/routes/index_stats.ts index d746de0a2878aa8..f0bad0977293489 100644 --- a/x-pack/legacy/plugins/lens/server/routes/index_stats.ts +++ b/x-pack/legacy/plugins/lens/server/routes/index_stats.ts @@ -13,6 +13,7 @@ import { IndexPatternsService, FieldDescriptor, } from '../../../../../../src/legacy/server/index_patterns/service'; +import { BASE_API_URL } from '../../common'; type Document = Record; @@ -22,7 +23,7 @@ export async function initStatsRoute(setup: CoreSetup) { const router = setup.http.createRouter(); router.post( { - path: '/index_stats/{indexPatternTitle}', + path: `${BASE_API_URL}/index_stats/{indexPatternTitle}`, validate: { params: schema.object({ indexPatternTitle: schema.string(), diff --git a/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts b/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts index 5cca26a290acae4..695326073dd67fc 100644 --- a/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts +++ b/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts @@ -8,7 +8,7 @@ import Boom from 'boom'; import { i18n } from '@kbn/i18n'; import { ServerRoute } from 'hapi'; import { KibanaConfig, SavedObjectsLegacyService } from 'src/legacy/server/kbn_server'; -import { HttpServiceSetup, Logger, PluginInitializerContext } from 'src/core/server'; +import { Logger, PluginInitializerContext, CoreSetup } from 'src/core/server'; import { ElasticsearchPlugin } from 'src/legacy/core_plugins/elasticsearch'; import { XPackMainPlugin } from '../../../xpack_main/xpack_main'; import { addLinksToSampleDatasets } from '../lib/sample_data_sets'; @@ -61,7 +61,8 @@ import { fileDataVisualizerRoutes } from '../routes/file_data_visualizer'; // @ts-ignore: could not find declaration file for module import { initMlServerLog, LogInitialization } from '../client/log'; -export interface MlHttpServiceSetup extends HttpServiceSetup { +type CoreHttpSetup = CoreSetup['http']; +export interface MlHttpServiceSetup extends CoreHttpSetup { route(route: ServerRoute | ServerRoute[]): void; }