move oss features registration to KP (#66524) (#66794)

mshustov · web-flow · commit e18895b77188 · 2020-05-16T09:04:33.000+02:00
* 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -11,7 +11,6 @@ const createSetup = (): jest.Mocked<PluginSetupContract> => {
     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
@@ -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
@@ -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<XPackInfo, 'license'>;
-  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<RecursiveReadonly<PluginSetupContract>> {
+    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<PluginStartContract> {
-    this.logger.debug('Starting plugin');
+  public start(core: CoreStart): RecursiveReadonly<PluginStartContract> {
+    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
@@ -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<any, any, any>;
@@ -53,24 +61,14 @@ 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];
   });
 
   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
@@ -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) =>
