From ee11ddc3d0c6c92c7a693c7d98699fd4cbd741fb Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Fri, 15 May 2020 20:01:33 +0200 Subject: [PATCH] move oss features registration to KP (#66524) * register OSS features with KP SO types only * use Licensing plugin API in features plugin * add plugin tests * filter hidden types out * cleanup tests * rename * add degug logging * add warning for setup contract * fix typo --- .../saved_objects_service.mock.ts | 1 + x-pack/legacy/plugins/xpack_main/index.js | 5 +- x-pack/plugins/endpoint/server/plugin.test.ts | 1 - x-pack/plugins/features/kibana.json | 1 + x-pack/plugins/features/server/mocks.ts | 1 - x-pack/plugins/features/server/plugin.test.ts | 97 +++++++++++++++++++ x-pack/plugins/features/server/plugin.ts | 72 +++++++------- .../features/server/routes/index.test.ts | 46 ++++----- .../plugins/features/server/routes/index.ts | 7 +- 9 files changed, 161 insertions(+), 70 deletions(-) create mode 100644 x-pack/plugins/features/server/plugin.test.ts diff --git a/src/core/server/saved_objects/saved_objects_service.mock.ts b/src/core/server/saved_objects/saved_objects_service.mock.ts index 4e1f5981d6a41..6f5ecb1eb464b 100644 --- a/src/core/server/saved_objects/saved_objects_service.mock.ts +++ b/src/core/server/saved_objects/saved_objects_service.mock.ts @@ -107,4 +107,5 @@ export const savedObjectsServiceMock = { createInternalStartContract: createInternalStartContractMock, createStartContract: createStartContractMock, createMigrationContext: migrationMocks.createContext, + createTypeRegistryMock: typeRegistryMock.create, }; diff --git a/x-pack/legacy/plugins/xpack_main/index.js b/x-pack/legacy/plugins/xpack_main/index.js index a4e255495fc86..3b79a6e4badb4 100644 --- a/x-pack/legacy/plugins/xpack_main/index.js +++ b/x-pack/legacy/plugins/xpack_main/index.js @@ -85,10 +85,7 @@ export const xpackMain = kibana => { mirrorPluginStatus(server.plugins.elasticsearch, this, 'yellow', 'red'); - featuresPlugin.registerLegacyAPI({ - xpackInfo: setupXPackMain(server), - savedObjectTypes: server.savedObjects.types, - }); + setupXPackMain(server); // register routes xpackInfoRoute(server); diff --git a/x-pack/plugins/endpoint/server/plugin.test.ts b/x-pack/plugins/endpoint/server/plugin.test.ts index 45e9591a14975..215b26942bcdb 100644 --- a/x-pack/plugins/endpoint/server/plugin.test.ts +++ b/x-pack/plugins/endpoint/server/plugin.test.ts @@ -34,7 +34,6 @@ describe('test endpoint plugin', () => { registerFeature: jest.fn(), getFeatures: jest.fn(), getFeaturesUICapabilities: jest.fn(), - registerLegacyAPI: jest.fn(), }; }); diff --git a/x-pack/plugins/features/kibana.json b/x-pack/plugins/features/kibana.json index 6e51f3b650710..1cab1821b1bf5 100644 --- a/x-pack/plugins/features/kibana.json +++ b/x-pack/plugins/features/kibana.json @@ -2,6 +2,7 @@ "id": "features", "version": "8.0.0", "kibanaVersion": "kibana", + "requiredPlugins": ["licensing"], "optionalPlugins": ["visTypeTimelion"], "configPath": ["xpack", "features"], "server": true, diff --git a/x-pack/plugins/features/server/mocks.ts b/x-pack/plugins/features/server/mocks.ts index ebaa5f1a504ca..d9437169a7453 100644 --- a/x-pack/plugins/features/server/mocks.ts +++ b/x-pack/plugins/features/server/mocks.ts @@ -11,7 +11,6 @@ const createSetup = (): jest.Mocked => { getFeatures: jest.fn(), getFeaturesUICapabilities: jest.fn(), registerFeature: jest.fn(), - registerLegacyAPI: jest.fn(), }; }; diff --git a/x-pack/plugins/features/server/plugin.test.ts b/x-pack/plugins/features/server/plugin.test.ts new file mode 100644 index 0000000000000..3d7cf19e58b0e --- /dev/null +++ b/x-pack/plugins/features/server/plugin.test.ts @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { coreMock, savedObjectsServiceMock } from 'src/core/server/mocks'; + +import { Plugin } from './plugin'; +const initContext = coreMock.createPluginInitializerContext(); +const coreSetup = coreMock.createSetup(); +const coreStart = coreMock.createStart(); +const typeRegistry = savedObjectsServiceMock.createTypeRegistryMock(); +typeRegistry.getAllTypes.mockReturnValue([ + { + name: 'foo', + hidden: false, + mappings: { properties: {} }, + namespaceType: 'single' as 'single', + }, + { + name: 'bar', + hidden: true, + mappings: { properties: {} }, + namespaceType: 'agnostic' as 'agnostic', + }, +]); +coreStart.savedObjects.getTypeRegistry.mockReturnValue(typeRegistry); + +describe('Features Plugin', () => { + it('returns OSS + registered features', async () => { + const plugin = new Plugin(initContext); + const { registerFeature } = await plugin.setup(coreSetup, {}); + registerFeature({ + id: 'baz', + name: 'baz', + app: [], + privileges: null, + }); + + const { getFeatures } = await plugin.start(coreStart); + + expect(getFeatures().map(f => f.id)).toMatchInlineSnapshot(` + Array [ + "baz", + "discover", + "visualize", + "dashboard", + "dev_tools", + "advancedSettings", + "indexPatterns", + "savedObjectsManagement", + ] + `); + }); + + it('returns OSS + registered features with timelion when available', async () => { + const plugin = new Plugin(initContext); + const { registerFeature } = await plugin.setup(coreSetup, { + visTypeTimelion: { uiEnabled: true }, + }); + registerFeature({ + id: 'baz', + name: 'baz', + app: [], + privileges: null, + }); + + const { getFeatures } = await plugin.start(coreStart); + + expect(getFeatures().map(f => f.id)).toMatchInlineSnapshot(` + Array [ + "baz", + "discover", + "visualize", + "dashboard", + "dev_tools", + "advancedSettings", + "indexPatterns", + "savedObjectsManagement", + "timelion", + ] + `); + }); + + it('registers not hidden saved objects types', async () => { + const plugin = new Plugin(initContext); + await plugin.setup(coreSetup, {}); + const { getFeatures } = await plugin.start(coreStart); + + const soTypes = + getFeatures().find(f => f.id === 'savedObjectsManagement')?.privileges?.all.savedObject.all || + []; + + expect(soTypes.includes('foo')).toBe(true); + expect(soTypes.includes('bar')).toBe(false); + }); +}); diff --git a/x-pack/plugins/features/server/plugin.ts b/x-pack/plugins/features/server/plugin.ts index 2405f05768a2f..e3480eda9fe7d 100644 --- a/x-pack/plugins/features/server/plugin.ts +++ b/x-pack/plugins/features/server/plugin.ts @@ -6,13 +6,14 @@ import { CoreSetup, + CoreStart, + SavedObjectsServiceStart, Logger, PluginInitializerContext, RecursiveReadonly, } from '../../../../src/core/server'; import { Capabilities as UICapabilities } from '../../../../src/core/server'; import { deepFreeze } from '../../../../src/core/server'; -import { XPackInfo } from '../../../legacy/plugins/xpack_main/server/lib/xpack_info'; import { PluginSetupContract as TimelionSetupContract } from '../../../../src/plugins/vis_type_timelion/server'; import { FeatureRegistry } from './feature_registry'; import { Feature, FeatureConfig } from '../common/feature'; @@ -25,39 +26,26 @@ import { defineRoutes } from './routes'; */ export interface PluginSetupContract { registerFeature(feature: FeatureConfig): void; + /* + * Calling this function during setup will crash Kibana. + * Use start contract instead. + * @deprecated + * */ getFeatures(): Feature[]; getFeaturesUICapabilities(): UICapabilities; - registerLegacyAPI: (legacyAPI: LegacyAPI) => void; } export interface PluginStartContract { getFeatures(): Feature[]; } -/** - * Describes a set of APIs that are available in the legacy platform only and required by this plugin - * to function properly. - */ -export interface LegacyAPI { - xpackInfo: Pick; - savedObjectTypes: string[]; -} - /** * Represents Features Plugin instance that will be managed by the Kibana plugin system. */ export class Plugin { private readonly logger: Logger; - private readonly featureRegistry: FeatureRegistry = new FeatureRegistry(); - - private legacyAPI?: LegacyAPI; - private readonly getLegacyAPI = () => { - if (!this.legacyAPI) { - throw new Error('Legacy API is not registered!'); - } - return this.legacyAPI; - }; + private isTimelionEnabled: boolean = false; constructor(private readonly initializerContext: PluginInitializerContext) { this.logger = this.initializerContext.logger.get(); @@ -67,39 +55,49 @@ export class Plugin { core: CoreSetup, { visTypeTimelion }: { visTypeTimelion?: TimelionSetupContract } ): Promise> { + this.isTimelionEnabled = visTypeTimelion !== undefined && visTypeTimelion.uiEnabled; + defineRoutes({ router: core.http.createRouter(), featureRegistry: this.featureRegistry, - getLegacyAPI: this.getLegacyAPI, }); return deepFreeze({ registerFeature: this.featureRegistry.register.bind(this.featureRegistry), getFeatures: this.featureRegistry.getAll.bind(this.featureRegistry), getFeaturesUICapabilities: () => uiCapabilitiesForFeatures(this.featureRegistry.getAll()), - - registerLegacyAPI: (legacyAPI: LegacyAPI) => { - this.legacyAPI = legacyAPI; - - // Register OSS features. - for (const feature of buildOSSFeatures({ - savedObjectTypes: this.legacyAPI.savedObjectTypes, - includeTimelion: visTypeTimelion !== undefined && visTypeTimelion.uiEnabled, - })) { - this.featureRegistry.register(feature); - } - }, }); } - public start(): RecursiveReadonly { - this.logger.debug('Starting plugin'); + public start(core: CoreStart): RecursiveReadonly { + this.registerOssFeatures(core.savedObjects); + return deepFreeze({ getFeatures: this.featureRegistry.getAll.bind(this.featureRegistry), }); } - public stop() { - this.logger.debug('Stopping plugin'); + public stop() {} + + private registerOssFeatures(savedObjects: SavedObjectsServiceStart) { + const registry = savedObjects.getTypeRegistry(); + const savedObjectTypes = registry + .getAllTypes() + .filter(t => !t.hidden) + .map(t => t.name); + + this.logger.debug( + `Registering OSS features with SO types: ${savedObjectTypes.join(', ')}. "includeTimelion": ${ + this.isTimelionEnabled + }.` + ); + const features = buildOSSFeatures({ + savedObjectTypes, + includeTimelion: this.isTimelionEnabled, + }); + + for (const feature of features) { + this.featureRegistry.register(feature); + } } } diff --git a/x-pack/plugins/features/server/routes/index.test.ts b/x-pack/plugins/features/server/routes/index.test.ts index c43e2a5195fe7..67b28b27f931f 100644 --- a/x-pack/plugins/features/server/routes/index.test.ts +++ b/x-pack/plugins/features/server/routes/index.test.ts @@ -7,12 +7,20 @@ import { FeatureRegistry } from '../feature_registry'; import { defineRoutes } from './index'; -import { httpServerMock, httpServiceMock } from '../../../../../src/core/server/mocks'; -import { XPackInfoLicense } from '../../../../legacy/plugins/xpack_main/server/lib/xpack_info_license'; +import { httpServerMock, httpServiceMock, coreMock } from '../../../../../src/core/server/mocks'; +import { LicenseType } from '../../../licensing/server/'; +import { licensingMock } from '../../../licensing/server/mocks'; import { RequestHandler } from '../../../../../src/core/server'; import { FeatureConfig } from '../../common'; -let currentLicenseLevel: string = 'gold'; +function createContextMock(licenseType: LicenseType = 'gold') { + return { + core: coreMock.createRequestHandlerContext(), + licensing: { + license: licensingMock.createLicense({ license: { type: licenseType } }), + }, + }; +} describe('GET /api/features', () => { let routeHandler: RequestHandler; @@ -53,16 +61,6 @@ describe('GET /api/features', () => { defineRoutes({ router: routerMock, featureRegistry, - getLegacyAPI: () => ({ - xpackInfo: { - license: { - isOneOf(candidateLicenses: string[]) { - return candidateLicenses.includes(currentLicenseLevel); - }, - } as XPackInfoLicense, - }, - savedObjectTypes: [], - }), }); routeHandler = routerMock.get.mock.calls[0][1]; @@ -70,7 +68,7 @@ describe('GET /api/features', () => { it('returns a list of available features, sorted by their configured order', async () => { const mockResponse = httpServerMock.createResponseFactory(); - routeHandler(undefined as any, { query: {} } as any, mockResponse); + routeHandler(createContextMock(), { query: {} } as any, mockResponse); expect(mockResponse.ok).toHaveBeenCalledTimes(1); const [call] = mockResponse.ok.mock.calls; @@ -98,10 +96,8 @@ describe('GET /api/features', () => { }); it(`by default does not return features that arent allowed by current license`, async () => { - currentLicenseLevel = 'basic'; - const mockResponse = httpServerMock.createResponseFactory(); - routeHandler(undefined as any, { query: {} } as any, mockResponse); + routeHandler(createContextMock('basic'), { query: {} } as any, mockResponse); expect(mockResponse.ok).toHaveBeenCalledTimes(1); const [call] = mockResponse.ok.mock.calls; @@ -126,10 +122,12 @@ describe('GET /api/features', () => { }); it(`ignoreValidLicenses=false does not return features that arent allowed by current license`, async () => { - currentLicenseLevel = 'basic'; - const mockResponse = httpServerMock.createResponseFactory(); - routeHandler(undefined as any, { query: { ignoreValidLicenses: false } } as any, mockResponse); + routeHandler( + createContextMock('basic'), + { query: { ignoreValidLicenses: false } } as any, + mockResponse + ); expect(mockResponse.ok).toHaveBeenCalledTimes(1); const [call] = mockResponse.ok.mock.calls; @@ -154,10 +152,12 @@ describe('GET /api/features', () => { }); it(`ignoreValidLicenses=true returns features that arent allowed by current license`, async () => { - currentLicenseLevel = 'basic'; - const mockResponse = httpServerMock.createResponseFactory(); - routeHandler(undefined as any, { query: { ignoreValidLicenses: true } } as any, mockResponse); + routeHandler( + createContextMock('basic'), + { query: { ignoreValidLicenses: true } } as any, + mockResponse + ); expect(mockResponse.ok).toHaveBeenCalledTimes(1); const [call] = mockResponse.ok.mock.calls; diff --git a/x-pack/plugins/features/server/routes/index.ts b/x-pack/plugins/features/server/routes/index.ts index 428500c3daa88..d07b488693091 100644 --- a/x-pack/plugins/features/server/routes/index.ts +++ b/x-pack/plugins/features/server/routes/index.ts @@ -6,7 +6,6 @@ import { schema } from '@kbn/config-schema'; import { IRouter } from '../../../../../src/core/server'; -import { LegacyAPI } from '../plugin'; import { FeatureRegistry } from '../feature_registry'; /** @@ -15,10 +14,9 @@ import { FeatureRegistry } from '../feature_registry'; export interface RouteDefinitionParams { router: IRouter; featureRegistry: FeatureRegistry; - getLegacyAPI: () => LegacyAPI; } -export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDefinitionParams) { +export function defineRoutes({ router, featureRegistry }: RouteDefinitionParams) { router.get( { path: '/api/features', @@ -37,7 +35,8 @@ export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDef request.query.ignoreValidLicenses || !feature.validLicenses || !feature.validLicenses.length || - getLegacyAPI().xpackInfo.license.isOneOf(feature.validLicenses) + (context.licensing!.license.type && + feature.validLicenses.includes(context.licensing!.license.type)) ) .sort( (f1, f2) =>