maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Enforce camelCase format for a plugin id (#53759) (#55270)

Browse source 
* add isCamelCase  function

* add a warning if id is not in camelCase

* document pluginId expected in camelCase

* regen docs

* add a test for logging

* update tests. warn can be called several times for different reasons

* pluginPath falls back to plugin id in snake_case

* update tests

* update docs

* add example with id & configPath different formats

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Mikhail Shustov 2020-01-18 15:44:32 +01:00 committed by GitHub
parent d70a4251bc
commit 666eda060b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 227 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
docs/development/core/server/kibana-plugin-server.discoveredplugin.configpath.md
View file

@ -4,7 +4,7 @@
## DiscoveredPlugin.configPath property
Root configuration path used by the plugin, defaults to "id".
Root configuration path used by the plugin, defaults to "id" in snake\_case format.
<b>Signature:</b>

2
docs/development/core/server/kibana-plugin-server.discoveredplugin.md
View file

@ -16,7 +16,7 @@ export interface DiscoveredPlugin
| Property | Type | Description |
| --- | --- | --- |
| [configPath](./kibana-plugin-server.discoveredplugin.configpath.md) | <code>ConfigPath</code> | Root configuration path used by the plugin, defaults to "id". |
| [configPath](./kibana-plugin-server.discoveredplugin.configpath.md) | <code>ConfigPath</code> | Root configuration path used by the plugin, defaults to "id" in snake\_case format. |
| [id](./kibana-plugin-server.discoveredplugin.id.md) | <code>PluginName</code> | Identifier of the plugin. |
| [optionalPlugins](./kibana-plugin-server.discoveredplugin.optionalplugins.md) | <code>readonly PluginName[]</code> | An optional list of the other plugins that if installed and enabled \*\*may be\*\* leveraged by this plugin for some additional functionality but otherwise are not required for this plugin to work properly. |
| [requiredPlugins](./kibana-plugin-server.discoveredplugin.requiredplugins.md) | <code>readonly PluginName[]</code> | An optional list of the other plugins that \*\*must be\*\* installed and enabled for this plugin to function properly. |

7
docs/development/core/server/kibana-plugin-server.pluginmanifest.configpath.md
View file

@ -4,10 +4,15 @@
## PluginManifest.configPath property
Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id".
Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id" in snake\_case format.
<b>Signature:</b>
```typescript
readonly configPath: ConfigPath;
```
## Example
id: myPlugin configPath: my\_plugin

2
docs/development/core/server/kibana-plugin-server.pluginmanifest.id.md
View file

@ -4,7 +4,7 @@
## PluginManifest.id property
Identifier of the plugin.
Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. Other plugins leverage it to access plugin API, navigate to the plugin, etc.
<b>Signature:</b>

4
docs/development/core/server/kibana-plugin-server.pluginmanifest.md
View file

@ -20,8 +20,8 @@ Should never be used in code outside of Core but is exported for documentation p
| Property | Type | Description |
| --- | --- | --- |
| [configPath](./kibana-plugin-server.pluginmanifest.configpath.md) | <code>ConfigPath</code> | Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id". |
| [id](./kibana-plugin-server.pluginmanifest.id.md) | <code>PluginName</code> | Identifier of the plugin. |
| [configPath](./kibana-plugin-server.pluginmanifest.configpath.md) | <code>ConfigPath</code> | Root [configuration path](./kibana-plugin-server.configpath.md) used by the plugin, defaults to "id" in snake\_case format. |
| [id](./kibana-plugin-server.pluginmanifest.id.md) | <code>PluginName</code> | Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract. Other plugins leverage it to access plugin API, navigate to the plugin, etc. |
| [kibanaVersion](./kibana-plugin-server.pluginmanifest.kibanaversion.md) | <code>string</code> | The version of Kibana the plugin is compatible with, defaults to "version". |
| [optionalPlugins](./kibana-plugin-server.pluginmanifest.optionalplugins.md) | <code>readonly PluginName[]</code> | An optional list of the other plugins that if installed and enabled \*\*may be\*\* leveraged by this plugin for some additional functionality but otherwise are not required for this plugin to work properly. |
| [requiredPlugins](./kibana-plugin-server.pluginmanifest.requiredplugins.md) | <code>readonly PluginName[]</code> | An optional list of the other plugins that \*\*must be\*\* installed and enabled for this plugin to function properly. |

1
src/core/CONVENTIONS.md
View file

@ -32,6 +32,7 @@ my_plugin/
   ├── index.ts
   └── plugin.ts
```
- [Manifest file](/docs/development/core/server/kibana-plugin-server.pluginmanifest.md) should be defined on top level.
- Both `server` and `public` should have an `index.ts` and a `plugin.ts` file:
- `index.ts` should only contain:
- The `plugin` export

25
src/core/server/config/integration_tests/config_deprecation.test.ts
View file

@ -36,7 +36,13 @@ describe('configuration deprecations', () => {
await root.setup();
const logs = loggingServiceMock.collect(mockLoggingService);
expect(logs.warn).toMatchInlineSnapshot(`Array []`);
const warnings = logs.warn.flatMap(i => i);
expect(warnings).not.toContain(
'"optimize.lazy" is deprecated and has been replaced by "optimize.watch"'
);
expect(warnings).not.toContain(
'"optimize.lazyPort" is deprecated and has been replaced by "optimize.watchPort"'
);
});
it('should log deprecation warnings for core deprecations', async () => {
@ -50,15 +56,12 @@ describe('configuration deprecations', () => {
await root.setup();
const logs = loggingServiceMock.collect(mockLoggingService);
expect(logs.warn).toMatchInlineSnapshot(`
Array [
Array [
"\\"optimize.lazy\\" is deprecated and has been replaced by \\"optimize.watch\\"",
],
Array [
"\\"optimize.lazyPort\\" is deprecated and has been replaced by \\"optimize.watchPort\\"",
],
]
`);
const warnings = logs.warn.flatMap(i => i);
expect(warnings).toContain(
'"optimize.lazy" is deprecated and has been replaced by "optimize.watch"'
);
expect(warnings).toContain(
'"optimize.lazyPort" is deprecated and has been replaced by "optimize.watchPort"'
);
});
});

42
src/core/server/plugins/discovery/is_camel_case.test.ts Normal file
View file

@ -0,0 +1,42 @@
/*
* 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 { isCamelCase } from './is_camel_case';
describe('isCamelCase', () => {
it('matches a string in camelCase', () => {
expect(isCamelCase('foo')).toBe(true);
expect(isCamelCase('foo1')).toBe(true);
expect(isCamelCase('fooBar')).toBe(true);
expect(isCamelCase('fooBarBaz')).toBe(true);
expect(isCamelCase('fooBAR')).toBe(true);
});
it('does not match strings in other cases', () => {
expect(isCamelCase('AAA')).toBe(false);
expect(isCamelCase('FooBar')).toBe(false);
expect(isCamelCase('3Foo')).toBe(false);
expect(isCamelCase('o_O')).toBe(false);
expect(isCamelCase('foo_bar')).toBe(false);
expect(isCamelCase('foo_')).toBe(false);
expect(isCamelCase('_fooBar')).toBe(false);
expect(isCamelCase('fooBar_')).toBe(false);
expect(isCamelCase('_fooBar_')).toBe(false);
});
});

22
src/core/server/plugins/discovery/is_camel_case.ts Normal file
View file

@ -0,0 +1,22 @@
/*
* 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 camelCaseRegExp = /^[a-z]{1}([a-zA-Z0-9]{1,})$/;
export function isCamelCase(candidate: string) {
return camelCaseRegExp.test(candidate);
}

164
src/core/server/plugins/discovery/plugin_manifest_parser.test.ts
View file

@ -20,10 +20,12 @@
import { PluginDiscoveryErrorType } from './plugin_discovery_error';
import { mockReadFile } from './plugin_manifest_parser.test.mocks';
import { loggingServiceMock } from '../../logging/logging_service.mock';
import { resolve } from 'path';
import { parseManifest } from './plugin_manifest_parser';
const logger = loggingServiceMock.createLogger();
const pluginPath = resolve('path', 'existent-dir');
const pluginManifestPath = resolve(pluginPath, 'kibana.json');
const packageInfo = {
@ -43,7 +45,7 @@ test('return error when manifest is empty', async () => {
cb(null, Buffer.from(''));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Unexpected end of JSON input (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
@ -55,7 +57,7 @@ test('return error when manifest content is null', async () => {
cb(null, Buffer.from('null'));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin manifest must contain a JSON encoded object. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
@ -67,7 +69,7 @@ test('return error when manifest content is not a valid JSON', async () => {
cb(null, Buffer.from('not-json'));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Unexpected token o in JSON at position 1 (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
@ -79,7 +81,7 @@ test('return error when plugin id is missing', async () => {
cb(null, Buffer.from(JSON.stringify({ version: 'some-version' })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin manifest must contain an "id" property. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
@ -91,20 +93,36 @@ test('return error when plugin id includes `.` characters', async () => {
cb(null, Buffer.from(JSON.stringify({ id: 'some.name', version: 'some-version' })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin "id" must not include \`.\` characters. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
});
test('return error when plugin version is missing', async () => {
test('logs warning if pluginId is not in camelCase format', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some-id' })));
cb(null, Buffer.from(JSON.stringify({ id: 'some_name', version: 'kibana', server: true })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin manifest for "some-id" must contain a "version" property. (invalid-manifest, ${pluginManifestPath})`,
expect(loggingServiceMock.collect(logger).warn).toHaveLength(0);
await parseManifest(pluginPath, packageInfo, logger);
expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(`
Array [
Array [
"Expect plugin \\"id\\" in camelCase, but found: some_name",
],
]
`);
});
test('return error when plugin version is missing', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'someId' })));
});
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin manifest for "someId" must contain a "version" property. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
@ -112,11 +130,11 @@ test('return error when plugin version is missing', async () => {
test('return error when plugin expected Kibana version is lower than actual version', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '6.4.2' })));
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '6.4.2' })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "some-id" is only compatible with Kibana version "6.4.2", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin "someId" is only compatible with Kibana version "6.4.2", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.IncompatibleVersion,
path: pluginManifestPath,
});
@ -126,12 +144,12 @@ test('return error when plugin expected Kibana version cannot be interpreted as
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Buffer.from(JSON.stringify({ id: 'some-id', version: '1.0.0', kibanaVersion: 'non-sem-ver' }))
Buffer.from(JSON.stringify({ id: 'someId', version: '1.0.0', kibanaVersion: 'non-sem-ver' }))
);
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "some-id" is only compatible with Kibana version "non-sem-ver", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin "someId" is only compatible with Kibana version "non-sem-ver", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.IncompatibleVersion,
path: pluginManifestPath,
});
@ -139,11 +157,11 @@ test('return error when plugin expected Kibana version cannot be interpreted as
test('return error when plugin config path is not a string', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', configPath: 2 })));
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', configPath: 2 })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `The "configPath" in plugin manifest for "some-id" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `The "configPath" in plugin manifest for "someId" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
@ -153,12 +171,12 @@ test('return error when plugin config path is an array that contains non-string
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', configPath: ['config', 2] }))
Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', configPath: ['config', 2] }))
);
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `The "configPath" in plugin manifest for "some-id" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `The "configPath" in plugin manifest for "someId" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
@ -166,11 +184,11 @@ test('return error when plugin config path is an array that contains non-string
test('return error when plugin expected Kibana version is higher than actual version', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.1' })));
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.1' })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "some-id" is only compatible with Kibana version "7.0.1", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Plugin "someId" is only compatible with Kibana version "7.0.1", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.IncompatibleVersion,
path: pluginManifestPath,
});
@ -178,11 +196,11 @@ test('return error when plugin expected Kibana version is higher than actual ver
test('return error when both `server` and `ui` are set to `false` or missing', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0' })));
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0' })));
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "some-id", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "someId", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
@ -190,12 +208,12 @@ test('return error when both `server` and `ui` are set to `false` or missing', a
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', server: false, ui: false }))
Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', server: false, ui: false }))
);
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "some-id", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "someId", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
@ -207,7 +225,7 @@ test('return error when manifest contains unrecognized properties', async () =>
null,
Buffer.from(
JSON.stringify({
id: 'some-id',
id: 'someId',
version: '7.0.0',
server: true,
unknownOne: 'one',
@ -217,21 +235,69 @@ test('return error when manifest contains unrecognized properties', async () =>
);
});
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Manifest for plugin "some-id" contains the following unrecognized properties: unknownOne,unknownTwo. (invalid-manifest, ${pluginManifestPath})`,
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
message: `Manifest for plugin "someId" contains the following unrecognized properties: unknownOne,unknownTwo. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
});
test('set defaults for all missing optional fields', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some-id', version: '7.0.0', server: true })));
describe('configPath', () => {
test('falls back to plugin id if not specified', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'plugin', version: '7.0.0', server: true })));
});
const manifest = await parseManifest(pluginPath, packageInfo, logger);
expect(manifest.configPath).toBe(manifest.id);
});
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'some-id',
configPath: 'some-id',
test('falls back to plugin id in snakeCase format', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'SomeId', version: '7.0.0', server: true })));
});
const manifest = await parseManifest(pluginPath, packageInfo, logger);
expect(manifest.configPath).toBe('some_id');
});
test('not formated to snakeCase if defined explicitly as string', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Buffer.from(
JSON.stringify({ id: 'someId', configPath: 'somePath', version: '7.0.0', server: true })
)
);
});
const manifest = await parseManifest(pluginPath, packageInfo, logger);
expect(manifest.configPath).toBe('somePath');
});
test('not formated to snakeCase if defined explicitly as an array of strings', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Buffer.from(
JSON.stringify({ id: 'someId', configPath: ['somePath'], version: '7.0.0', server: true })
)
);
});
const manifest = await parseManifest(pluginPath, packageInfo, logger);
expect(manifest.configPath).toEqual(['somePath']);
});
});
test('set defaults for all missing optional fields', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', server: true })));
});
await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
id: 'someId',
configPath: 'some_id',
version: '7.0.0',
kibanaVersion: '7.0.0',
optionalPlugins: [],
@ -247,7 +313,7 @@ test('return all set optional fields as they are in manifest', async () => {
null,
Buffer.from(
JSON.stringify({
id: 'some-id',
id: 'someId',
configPath: ['some', 'path'],
version: 'some-version',
kibanaVersion: '7.0.0',
@ -259,8 +325,8 @@ test('return all set optional fields as they are in manifest', async () => {
);
});
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'some-id',
await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
id: 'someId',
configPath: ['some', 'path'],
version: 'some-version',
kibanaVersion: '7.0.0',
@ -277,7 +343,7 @@ test('return manifest when plugin expected Kibana version matches actual version
null,
Buffer.from(
JSON.stringify({
id: 'some-id',
id: 'someId',
configPath: 'some-path',
version: 'some-version',
kibanaVersion: '7.0.0-alpha2',
@ -288,8 +354,8 @@ test('return manifest when plugin expected Kibana version matches actual version
);
});
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'some-id',
await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
id: 'someId',
configPath: 'some-path',
version: 'some-version',
kibanaVersion: '7.0.0-alpha2',
@ -306,7 +372,7 @@ test('return manifest when plugin expected Kibana version is `kibana`', async ()
null,
Buffer.from(
JSON.stringify({
id: 'some-id',
id: 'someId',
version: 'some-version',
kibanaVersion: 'kibana',
requiredPlugins: ['some-required-plugin'],
@ -317,9 +383,9 @@ test('return manifest when plugin expected Kibana version is `kibana`', async ()
);
});
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'some-id',
configPath: 'some-id',
await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
id: 'someId',
configPath: 'some_id',
version: 'some-version',
kibanaVersion: 'kibana',
optionalPlugins: [],

11
src/core/server/plugins/discovery/plugin_manifest_parser.ts
View file

@ -21,9 +21,12 @@ import { readFile, stat } from 'fs';
import { resolve } from 'path';
import { coerce } from 'semver';
import { promisify } from 'util';
import { snakeCase } from 'lodash';
import { isConfigPath, PackageInfo } from '../../config';
import { Logger } from '../../logging';
import { PluginManifest } from '../types';
import { PluginDiscoveryError } from './plugin_discovery_error';
import { isCamelCase } from './is_camel_case';
const fsReadFileAsync = promisify(readFile);
const fsStatAsync = promisify(stat);
@ -67,7 +70,7 @@ const KNOWN_MANIFEST_FIELDS = (() => {
* @param packageInfo Kibana package info.
* @internal
*/
export async function parseManifest(pluginPath: string, packageInfo: PackageInfo) {
export async function parseManifest(pluginPath: string, packageInfo: PackageInfo, log: Logger) {
const manifestPath = resolve(pluginPath, MANIFEST_FILE_NAME);
let manifestContent;
@ -107,6 +110,10 @@ export async function parseManifest(pluginPath: string, packageInfo: PackageInfo
);
}
if (!isCamelCase(manifest.id)) {
log.warn(`Expect plugin "id" in camelCase, but found: ${manifest.id}`);
}
if (!manifest.version || typeof manifest.version !== 'string') {
throw PluginDiscoveryError.invalidManifest(
manifestPath,
@ -161,7 +168,7 @@ export async function parseManifest(pluginPath: string, packageInfo: PackageInfo
id: manifest.id,
version: manifest.version,
kibanaVersion: expectedKibanaVersion,
configPath: manifest.configPath || manifest.id,
configPath: manifest.configPath || snakeCase(manifest.id),
requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [],
optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [],
ui: includesUiPlugin,

2
src/core/server/plugins/discovery/plugins_discovery.ts
View file

@ -112,7 +112,7 @@ function processPluginSearchPaths$(pluginDirs: readonly string[], log: Logger) {
* @param coreContext Kibana core context.
*/
function createPlugin$(path: string, log: Logger, coreContext: CoreContext) {
return from(parseManifest(path, coreContext.env.packageInfo)).pipe(
return from(parseManifest(path, coreContext.env.packageInfo, log)).pipe(
map(manifest => {
log.debug(`Successfully discovered plugin "${manifest.id}" at "${path}"`);
const opaqueId = Symbol(manifest.id);

11
src/core/server/plugins/types.ts
View file

@ -105,7 +105,8 @@ export type PluginOpaqueId = symbol;
*/
export interface PluginManifest {
/**
* Identifier of the plugin.
* Identifier of the plugin. Must be a string in camelCase. Part of a plugin public contract.
* Other plugins leverage it to access plugin API, navigate to the plugin, etc.
*/
readonly id: PluginName;
@ -121,7 +122,11 @@ export interface PluginManifest {
/**
* Root {@link ConfigPath | configuration path} used by the plugin, defaults
* to "id".
* to "id" in snake_case format.
*
* @example
* id: myPlugin
* configPath: my_plugin
*/
readonly configPath: ConfigPath;
@ -162,7 +167,7 @@ export interface DiscoveredPlugin {
readonly id: PluginName;
/**
* Root configuration path used by the plugin, defaults to "id".
* Root configuration path used by the plugin, defaults to "id" in snake_case format.
*/
readonly configPath: ConfigPath;

8
src/core/server/server.api.md
View file

@ -2005,9 +2005,9 @@ export const validBodyOutput: readonly ["data", "stream"];
// src/core/server/legacy/types.ts:161:3 - (ae-forgotten-export) The symbol "LegacyAppSpec" needs to be exported by the entry point index.d.ts
// src/core/server/legacy/types.ts:162:16 - (ae-forgotten-export) The symbol "LegacyPluginSpec" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/plugins_service.ts:43:5 - (ae-forgotten-export) The symbol "InternalPluginInfo" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:221:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:221:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:222:3 - (ae-forgotten-export) The symbol "ElasticsearchConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:223:3 - (ae-forgotten-export) The symbol "PathConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:226:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:227:3 - (ae-forgotten-export) The symbol "ElasticsearchConfigType" needs to be exported by the entry point index.d.ts
// src/core/server/plugins/types.ts:228:3 - (ae-forgotten-export) The symbol "PathConfigType" needs to be exported by the entry point index.d.ts
```