Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new platform plugin discovery. #24256

Merged
merged 20 commits into from
Nov 7, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6afb473
Implement new platform plugin discovery.
azasypkin Oct 5, 2018
ae8d61b
Review#1: Make `discover` to accept single `PluginsConfig` instance i…
azasypkin Oct 22, 2018
b3eaf91
Merge branch 'master' into issue-xxx-core-plugin-discovery
azasypkin Oct 26, 2018
1a047e5
Make plugin manifest interface "immutable".
azasypkin Oct 26, 2018
c4819ae
Merge branch 'master' into issue-xxx-core-plugin-discovery
azasypkin Nov 1, 2018
a4e8229
Merge branch 'master' into issue-xxx-core-plugin-discovery
azasypkin Nov 2, 2018
8dfe3be
Review №2: make tests platform agnostic.
azasypkin Nov 2, 2018
aff6a97
Review №2: make `cause` required parameter for `PluginDiscoveryError`.
azasypkin Nov 2, 2018
6257e41
Review №2: use `discover` function instead of `PluginDiscovery` class.
azasypkin Nov 2, 2018
b2e1ea1
Review №2: rename `plugins$/errors$` to `plugin$/error$`.
azasypkin Nov 2, 2018
dd5536b
Review №2: improve `createPlugin$` JSDoc.
azasypkin Nov 2, 2018
8e836b9
Review №2: throw more specific errors while parsing plugin manifest.
azasypkin Nov 2, 2018
c83f084
Review №2: log errors before discovery result message.
azasypkin Nov 2, 2018
89a85ea
Add version compatibility check.
azasypkin Nov 5, 2018
ab2e9c2
Merge branch 'master' into issue-xxx-core-plugin-discovery
azasypkin Nov 5, 2018
c361f81
Fix unit test.
azasypkin Nov 5, 2018
82e470b
Review №3: use `toEqual` instead of `toMatchInlineSnapshot` for the t…
azasypkin Nov 6, 2018
52290ee
Review №3: move manifest parser tests to a separate file + switch man…
azasypkin Nov 6, 2018
c2af2d3
Review №3: add basic `PluginService` test.
azasypkin Nov 6, 2018
f5e424a
Merge branch 'master' into issue-xxx-core-plugin-discovery
azasypkin Nov 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/core/server/config/__snapshots__/env.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ Env {
"configs": Array [
"/some/other/path/some-kibana.yml",
],
"corePluginsDir": "/test/cwd/core_plugins",
"homeDir": "/test/cwd",
"isDevClusterMaster": false,
"logDir": "/test/cwd/log",
Expand Down Expand Up @@ -53,7 +52,6 @@ Env {
"configs": Array [
"/some/other/path/some-kibana.yml",
],
"corePluginsDir": "/test/cwd/core_plugins",
"homeDir": "/test/cwd",
"isDevClusterMaster": false,
"logDir": "/test/cwd/log",
Expand Down Expand Up @@ -88,7 +86,6 @@ Env {
"configs": Array [
"/test/cwd/config/kibana.yml",
],
"corePluginsDir": "/test/cwd/core_plugins",
"homeDir": "/test/cwd",
"isDevClusterMaster": true,
"logDir": "/test/cwd/log",
Expand Down Expand Up @@ -123,7 +120,6 @@ Env {
"configs": Array [
"/some/other/path/some-kibana.yml",
],
"corePluginsDir": "/test/cwd/core_plugins",
"homeDir": "/test/cwd",
"isDevClusterMaster": false,
"logDir": "/test/cwd/log",
Expand Down Expand Up @@ -158,7 +154,6 @@ Env {
"configs": Array [
"/some/other/path/some-kibana.yml",
],
"corePluginsDir": "/test/cwd/core_plugins",
"homeDir": "/test/cwd",
"isDevClusterMaster": false,
"logDir": "/test/cwd/log",
Expand Down Expand Up @@ -193,7 +188,6 @@ Env {
"configs": Array [
"/some/other/path/some-kibana.yml",
],
"corePluginsDir": "/some/home/dir/core_plugins",
"homeDir": "/some/home/dir",
"isDevClusterMaster": false,
"logDir": "/some/home/dir/log",
Expand Down
2 changes: 0 additions & 2 deletions src/core/server/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ export class Env {
}

public readonly configDir: string;
public readonly corePluginsDir: string;
public readonly binDir: string;
public readonly logDir: string;
public readonly staticFilesDir: string;
Expand Down Expand Up @@ -96,7 +95,6 @@ export class Env {
*/
constructor(readonly homeDir: string, options: EnvOptions) {
this.configDir = resolve(this.homeDir, 'config');
this.corePluginsDir = resolve(this.homeDir, 'core_plugins');
this.binDir = resolve(this.homeDir, 'bin');
this.logDir = resolve(this.homeDir, 'log');
this.staticFilesDir = resolve(this.homeDir, 'ui');
Expand Down
11 changes: 11 additions & 0 deletions src/core/server/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ jest.mock('./http/http_service', () => ({
HttpService: jest.fn(() => mockHttpService),
}));

const mockPluginsService = { start: jest.fn(), stop: jest.fn() };
jest.mock('./plugins/plugins_service', () => ({
PluginsService: jest.fn(() => mockPluginsService),
}));

const mockLegacyService = { start: jest.fn(), stop: jest.fn() };
jest.mock('./legacy_compat/legacy_service', () => ({
LegacyService: jest.fn(() => mockLegacyService),
Expand All @@ -45,6 +50,8 @@ afterEach(() => {
mockConfigService.atPath.mockReset();
mockHttpService.start.mockReset();
mockHttpService.stop.mockReset();
mockPluginsService.start.mockReset();
mockPluginsService.stop.mockReset();
mockLegacyService.start.mockReset();
mockLegacyService.stop.mockReset();
});
Expand All @@ -56,11 +63,13 @@ test('starts services on "start"', async () => {
const server = new Server(mockConfigService as any, logger, env);

expect(mockHttpService.start).not.toHaveBeenCalled();
expect(mockPluginsService.start).not.toHaveBeenCalled();
expect(mockLegacyService.start).not.toHaveBeenCalled();

await server.start();

expect(mockHttpService.start).toHaveBeenCalledTimes(1);
expect(mockPluginsService.start).toHaveBeenCalledTimes(1);
expect(mockLegacyService.start).toHaveBeenCalledTimes(1);
expect(mockLegacyService.start).toHaveBeenCalledWith(mockHttpServiceStartContract);
});
Expand Down Expand Up @@ -112,10 +121,12 @@ test('stops services on "stop"', async () => {
await server.start();

expect(mockHttpService.stop).not.toHaveBeenCalled();
expect(mockPluginsService.stop).not.toHaveBeenCalled();
expect(mockLegacyService.stop).not.toHaveBeenCalled();

await server.stop();

expect(mockHttpService.stop).toHaveBeenCalledTimes(1);
expect(mockPluginsService.stop).toHaveBeenCalledTimes(1);
expect(mockLegacyService.stop).toHaveBeenCalledTimes(1);
});
6 changes: 6 additions & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
* under the License.
*/

import { PluginsModule } from './plugins';

export { bootstrap } from './bootstrap';

import { first } from 'rxjs/operators';
Expand All @@ -27,6 +29,7 @@ import { Logger, LoggerFactory } from './logging';

export class Server {
private readonly http: HttpModule;
private readonly plugins: PluginsModule;
private readonly legacy: LegacyCompatModule;
private readonly log: Logger;

Expand All @@ -38,6 +41,7 @@ export class Server {
this.log = logger.get('server');

this.http = new HttpModule(configService.atPath('server', HttpConfig), logger);
this.plugins = new PluginsModule(configService, logger, env);
this.legacy = new LegacyCompatModule(configService, logger, env);
}

Expand All @@ -54,6 +58,7 @@ export class Server {
httpServerInfo = await this.http.service.start();
}

await this.plugins.service.start();
await this.legacy.service.start(httpServerInfo);

const unhandledConfigPaths = await this.configService.getUnusedPaths();
Expand All @@ -70,6 +75,7 @@ export class Server {
this.log.debug('stopping server');

await this.legacy.service.stop();
await this.plugins.service.stop();
await this.http.service.stop();
}
}
30 changes: 30 additions & 0 deletions src/core/server/plugins/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { ConfigService, Env } from '../config';
import { LoggerFactory } from '../logging';
import { PluginsService } from './plugins_service';

export class PluginsModule {
public readonly service: PluginsService;

constructor(private readonly configService: ConfigService, logger: LoggerFactory, env: Env) {
this.service = new PluginsService(env, logger, this.configService);
}
}
139 changes: 139 additions & 0 deletions src/core/server/plugins/plugin_discovery.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

const mockReaddir = jest.fn();
const mockReadFile = jest.fn();
const mockStat = jest.fn();
jest.mock('fs', () => ({
readdir: mockReaddir,
readFile: mockReadFile,
stat: mockStat,
}));

import { of } from 'rxjs';
import { map, toArray } from 'rxjs/operators';
import { logger } from '../logging/__mocks__';
import { PluginsDiscovery } from './plugins_discovery';

beforeEach(() => {
mockReaddir.mockImplementation((path, cb) => {
if (path === '/scan/non-empty/') {
spalger marked this conversation as resolved.
Show resolved Hide resolved
cb(null, ['1', '2-no-manifest', '3', '4-incomplete-manifest']);
} else if (path === '/scan/non-empty-2/') {
cb(null, ['5-invalid-manifest', '6', '7-non-dir']);
} else if (path.includes('non-existent')) {
cb(new Error('ENOENT'));
} else {
cb(null, []);
}
});

mockStat.mockImplementation((path, cb) => {
if (path.includes('non-existent')) {
cb(new Error('ENOENT'));
} else {
cb(null, { isDirectory: () => !path.includes('non-dir') });
}
});

mockReadFile.mockImplementation((path, cb) => {
if (path.endsWith('no-manifest/kibana.json')) {
cb(new Error('ENOENT'));
} else if (path.endsWith('invalid-manifest/kibana.json')) {
cb(null, Buffer.from('not-json'));
} else if (path.endsWith('incomplete-manifest/kibana.json')) {
cb(null, Buffer.from(JSON.stringify({ version: '1' })));
} else {
cb(null, Buffer.from(JSON.stringify({ id: 'plugin', version: '1' })));
}
});
});

afterEach(() => {
jest.clearAllMocks();
});

test('properly scans folders and paths', async () => {
const discovery = new PluginsDiscovery(logger.get());
const { plugins$, errors$ } = discovery.discover(
of({
initialize: true,
scanDirs: ['/scan/non-empty/', '/scan/non-existent/', '/scan/empty/', '/scan/non-empty-2/'],
paths: ['/path/existent-dir', '/path/non-dir', '/path/non-existent', '/path/existent-dir-2'],
})
);

await expect(plugins$.pipe(toArray()).toPromise()).resolves.toMatchInlineSnapshot(`
Array [
Object {
"manifest": Object {
"id": "plugin",
"version": "1",
},
"path": "/scan/non-empty/1",
},
Object {
"manifest": Object {
"id": "plugin",
"version": "1",
},
"path": "/scan/non-empty/3",
},
Object {
"manifest": Object {
"id": "plugin",
"version": "1",
},
"path": "/scan/non-empty-2/6",
},
Object {
"manifest": Object {
"id": "plugin",
"version": "1",
},
"path": "/path/existent-dir",
},
Object {
"manifest": Object {
"id": "plugin",
"version": "1",
},
"path": "/path/existent-dir-2",
},
]
`);

await expect(
errors$
.pipe(
map(error => error.toString()),
toArray()
)
.toPromise()
).resolves.toMatchInlineSnapshot(`
Array [
"Error: ENOENT (missing-manifest, /scan/non-empty/2-no-manifest/kibana.json)",
"Error: The \\"id\\" or/and \\"version\\" is missing in the plugin manifest. (invalid-manifest, /scan/non-empty/4-incomplete-manifest/kibana.json)",
"Error: ENOENT (invalid-scan-dir, /scan/non-existent/)",
"Error: Unexpected token o in JSON at position 1 (invalid-manifest, /scan/non-empty-2/5-invalid-manifest/kibana.json)",
"Error: /path/non-dir is not a directory. (invalid-plugin-dir, /path/non-dir)",
"Error: ENOENT (invalid-plugin-dir, /path/non-existent)",
]
`);
});
43 changes: 43 additions & 0 deletions src/core/server/plugins/plugin_discovery_error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export enum PluginDiscoveryErrorType {
spalger marked this conversation as resolved.
Show resolved Hide resolved
InvalidScanDirectory = 'invalid-scan-dir',
InvalidPluginDirectory = 'invalid-plugin-dir',
InvalidManifest = 'invalid-manifest',
MissingManifest = 'missing-manifest',
}

export class PluginDiscoveryError extends Error {
constructor(
public readonly type: PluginDiscoveryErrorType,
public readonly path: string,
public readonly cause?: Error
spalger marked this conversation as resolved.
Show resolved Hide resolved
) {
super(cause && cause.message);

// Set the prototype explicitly, see:
// https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work
Object.setPrototypeOf(this, PluginDiscoveryError.prototype);
}

public toString() {
return `${super.toString()} (${this.type}, ${this.path})`;
}
}
Loading