Skip to content

Commit

Permalink
move oss features registration to KP (elastic#66524)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mshustov committed May 15, 2020
1 parent 36ab9d3 commit ee11ddc
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,5 @@ export const savedObjectsServiceMock = {
createInternalStartContract: createInternalStartContractMock,
createStartContract: createStartContractMock,
createMigrationContext: migrationMocks.createContext,
createTypeRegistryMock: typeRegistryMock.create,
};
5 changes: 1 addition & 4 deletions x-pack/legacy/plugins/xpack_main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/endpoint/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ describe('test endpoint plugin', () => {
registerFeature: jest.fn(),
getFeatures: jest.fn(),
getFeaturesUICapabilities: jest.fn(),
registerLegacyAPI: jest.fn(),
};
});

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/features/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"id": "features",
"version": "8.0.0",
"kibanaVersion": "kibana",
"requiredPlugins": ["licensing"],
"optionalPlugins": ["visTypeTimelion"],
"configPath": ["xpack", "features"],
"server": true,
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/features/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const createSetup = (): jest.Mocked<PluginSetupContract> => {
getFeatures: jest.fn(),
getFeaturesUICapabilities: jest.fn(),
registerFeature: jest.fn(),
registerLegacyAPI: jest.fn(),
};
};

Expand Down
97 changes: 97 additions & 0 deletions x-pack/plugins/features/server/plugin.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
72 changes: 35 additions & 37 deletions x-pack/plugins/features/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();
Expand All @@ -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);
}
}
}
46 changes: 23 additions & 23 deletions x-pack/plugins/features/server/routes/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions x-pack/plugins/features/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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',
Expand All @@ -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) =>
Expand Down

0 comments on commit ee11ddc

Please sign in to comment.