Allow custom NP plugin paths in production (#53562)

* allow NP plugins --plugin-path in production, logs a warning

* remove env.staticFilesDir (unused)

* only show warning in development

* update rendering tests snapshots

* fix typo
This commit is contained in:
Pierre Gayvallet 2020-01-06 09:24:15 +01:00 committed by GitHub
parent c19151d474
commit 24542c3ad7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 187 additions and 70 deletions

View file

@ -140,23 +140,12 @@ function applyConfigOverrides(rawConfig, opts, extraCliOptions) {
}
set('plugins.scanDirs', _.compact([].concat(get('plugins.scanDirs'), opts.pluginDir)));
set(
'plugins.paths',
_.compact(
[].concat(
get('plugins.paths'),
opts.pluginPath,
opts.runExamples
? [
// Ideally this would automatically include all plugins in the examples dir
fromRoot('examples/demo_search'),
fromRoot('examples/search_explorer'),
fromRoot('examples/embeddable_examples'),
fromRoot('examples/embeddable_explorer'),
]
: [],
XPACK_INSTALLED && !opts.oss ? [XPACK_DIR] : []
)
)
@ -253,6 +242,7 @@ export default function(program) {
silent: !!opts.silent,
watch: !!opts.watch,
repl: !!opts.repl,
runExamples: !!opts.runExamples,
// We want to run without base path when the `--run-examples` flag is given so that we can use local
// links in other documentation sources, like "View this tutorial [here](http://localhost:5601/app/tutorial/xyz)".
// We can tell users they only have to run with `yarn start --run-examples` to get those

View file

@ -38,6 +38,7 @@ export function getEnvOptions(options: DeepPartial<EnvOptions> = {}): EnvOptions
basePath: false,
optimize: false,
oss: false,
runExamples: false,
...(options.cliArgs || {}),
},
isDevClusterMaster:

View file

@ -12,6 +12,7 @@ Env {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -40,7 +41,6 @@ Env {
"/test/kibanaRoot/plugins",
"/test/kibanaRoot/../kibana-extra",
],
"staticFilesDir": "/test/kibanaRoot/ui",
}
`;
@ -56,6 +56,7 @@ Env {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -84,7 +85,6 @@ Env {
"/test/kibanaRoot/plugins",
"/test/kibanaRoot/../kibana-extra",
],
"staticFilesDir": "/test/kibanaRoot/ui",
}
`;
@ -99,6 +99,7 @@ Env {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -127,7 +128,6 @@ Env {
"/test/kibanaRoot/plugins",
"/test/kibanaRoot/../kibana-extra",
],
"staticFilesDir": "/test/kibanaRoot/ui",
}
`;
@ -142,6 +142,7 @@ Env {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -170,7 +171,6 @@ Env {
"/test/kibanaRoot/plugins",
"/test/kibanaRoot/../kibana-extra",
],
"staticFilesDir": "/test/kibanaRoot/ui",
}
`;
@ -185,6 +185,7 @@ Env {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -213,7 +214,6 @@ Env {
"/test/kibanaRoot/plugins",
"/test/kibanaRoot/../kibana-extra",
],
"staticFilesDir": "/test/kibanaRoot/ui",
}
`;
@ -228,6 +228,7 @@ Env {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -256,6 +257,5 @@ Env {
"/some/home/dir/plugins",
"/some/home/dir/../kibana-extra",
],
"staticFilesDir": "/some/home/dir/ui",
}
`;

View file

@ -152,3 +152,25 @@ test('pluginSearchPaths does not contains x-pack plugins path if --oss flag is t
expect(env.pluginSearchPaths).not.toContain('/some/home/dir/x-pack/plugins');
});
test('pluginSearchPaths contains examples plugins path if --run-examples flag is true', () => {
const env = new Env(
'/some/home/dir',
getEnvOptions({
cliArgs: { runExamples: true },
})
);
expect(env.pluginSearchPaths).toContain('/some/home/dir/examples');
});
test('pluginSearchPaths does not contains examples plugins path if --run-examples flag is false', () => {
const env = new Env(
'/some/home/dir',
getEnvOptions({
cliArgs: { runExamples: false },
})
);
expect(env.pluginSearchPaths).not.toContain('/some/home/dir/examples');
});

View file

@ -43,6 +43,7 @@ export interface CliArgs {
optimize: boolean;
open: boolean;
oss: boolean;
runExamples: boolean;
}
export class Env {
@ -61,8 +62,6 @@ export class Env {
/** @internal */
public readonly logDir: string;
/** @internal */
public readonly staticFilesDir: string;
/** @internal */
public readonly pluginSearchPaths: readonly string[];
/**
@ -100,14 +99,14 @@ export class Env {
this.configDir = resolve(this.homeDir, 'config');
this.binDir = resolve(this.homeDir, 'bin');
this.logDir = resolve(this.homeDir, 'log');
this.staticFilesDir = resolve(this.homeDir, 'ui');
this.pluginSearchPaths = [
resolve(this.homeDir, 'src', 'plugins'),
options.cliArgs.oss ? '' : resolve(this.homeDir, 'x-pack', 'plugins'),
...(options.cliArgs.oss ? [] : [resolve(this.homeDir, 'x-pack', 'plugins')]),
resolve(this.homeDir, 'plugins'),
...(options.cliArgs.runExamples ? [resolve(this.homeDir, 'examples')] : []),
resolve(this.homeDir, '..', 'kibana-extra'),
].filter(Boolean);
];
this.cliArgs = Object.freeze(options.cliArgs);
this.configs = Object.freeze(options.configs);

View file

@ -19,8 +19,6 @@
import uuid from 'uuid';
import { config, HttpConfig } from '.';
import { Env } from '../config';
import { getEnvOptions } from '../config/__mocks__/env';
const validHostnames = ['www.example.com', '8.8.8.8', '::1', 'localhost'];
const invalidHostname = 'asdf$%^';
@ -265,8 +263,7 @@ describe('with TLS', () => {
clientAuthentication: 'none',
},
}),
{} as any,
Env.createDefault(getEnvOptions())
{} as any
);
expect(httpConfig.ssl.requestCert).toBe(false);
@ -283,8 +280,7 @@ describe('with TLS', () => {
clientAuthentication: 'optional',
},
}),
{} as any,
Env.createDefault(getEnvOptions())
{} as any
);
expect(httpConfig.ssl.requestCert).toBe(true);
@ -301,8 +297,7 @@ describe('with TLS', () => {
clientAuthentication: 'required',
},
}),
{} as any,
Env.createDefault(getEnvOptions())
{} as any
);
expect(httpConfig.ssl.requestCert).toBe(true);

View file

@ -18,7 +18,6 @@
*/
import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema';
import { Env } from '../config';
import { CspConfigType, CspConfig, ICspConfig } from '../csp';
import { SslConfig, sslSchema } from './ssl_config';
@ -135,7 +134,6 @@ export class HttpConfig {
public maxPayload: ByteSizeValue;
public basePath?: string;
public rewriteBasePath: boolean;
public publicDir: string;
public defaultRoute?: string;
public ssl: SslConfig;
public compression: { enabled: boolean; referrerWhitelist?: string[] };
@ -144,7 +142,7 @@ export class HttpConfig {
/**
* @internal
*/
constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType, env: Env) {
constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType) {
this.autoListen = rawHttpConfig.autoListen;
this.host = rawHttpConfig.host;
this.port = rawHttpConfig.port;
@ -154,7 +152,6 @@ export class HttpConfig {
this.keepaliveTimeout = rawHttpConfig.keepaliveTimeout;
this.socketTimeout = rawHttpConfig.socketTimeout;
this.rewriteBasePath = rawHttpConfig.rewriteBasePath;
this.publicDir = env.staticFilesDir;
this.ssl = new SslConfig(rawHttpConfig.ssl || {});
this.defaultRoute = rawHttpConfig.defaultRoute;
this.compression = rawHttpConfig.compression;

View file

@ -61,14 +61,14 @@ export class HttpService implements CoreService<InternalHttpServiceSetup, HttpSe
private requestHandlerContext?: RequestHandlerContextContainer;
constructor(private readonly coreContext: CoreContext) {
const { logger, configService, env } = coreContext;
const { logger, configService } = coreContext;
this.logger = logger;
this.log = logger.get('http');
this.config$ = combineLatest(
configService.atPath<HttpConfigType>(httpConfig.path),
configService.atPath<CspConfigType>(cspConfig.path)
).pipe(map(([http, csp]) => new HttpConfig(http, csp, env)));
).pipe(map(([http, csp]) => new HttpConfig(http, csp)));
this.httpServer = new HttpServer(logger, 'Kibana');
this.httpsRedirectServer = new HttpsRedirectServer(logger.get('http', 'redirect', 'server'));
}

View file

@ -31,8 +31,6 @@ import { HttpConfig, config } from './http_config';
import { Router } from './router';
import { loggingServiceMock } from '../logging/logging_service.mock';
import { ByteSizeValue } from '@kbn/config-schema';
import { Env } from '../config';
import { getEnvOptions } from '../config/__mocks__/env';
const emptyOutput = {
statusCode: 400,
@ -122,8 +120,7 @@ describe('getServerOptions', () => {
certificate: 'some-certificate-path',
},
}),
{} as any,
Env.createDefault(getEnvOptions())
{} as any
);
expect(getServerOptions(httpConfig).tls).toMatchInlineSnapshot(`
@ -152,8 +149,7 @@ describe('getServerOptions', () => {
clientAuthentication: 'required',
},
}),
{} as any,
Env.createDefault(getEnvOptions())
{} as any
);
expect(getServerOptions(httpConfig).tls).toMatchInlineSnapshot(`

View file

@ -84,7 +84,7 @@ export class LegacyService implements CoreService {
private settings?: LegacyVars;
constructor(private readonly coreContext: CoreContext) {
const { logger, configService, env } = coreContext;
const { logger, configService } = coreContext;
this.log = logger.get('legacy-service');
this.devConfig$ = configService
@ -93,7 +93,7 @@ export class LegacyService implements CoreService {
this.httpConfig$ = combineLatest(
configService.atPath<HttpConfigType>(httpConfig.path),
configService.atPath<CspConfigType>(cspConfig.path)
).pipe(map(([http, csp]) => new HttpConfig(http, csp, env)));
).pipe(map(([http, csp]) => new HttpConfig(http, csp)));
}
public async discoverPlugins(): Promise<LegacyServiceDiscoverPlugins> {

View file

@ -18,13 +18,14 @@
*/
import { mockPackage, mockReaddir, mockReadFile, mockStat } from './plugins_discovery.test.mocks';
import { rawConfigServiceMock } from '../../config/raw_config_service.mock';
import { loggingServiceMock } from '../../logging/logging_service.mock';
import { resolve } from 'path';
import { first, map, toArray } from 'rxjs/operators';
import { ConfigService, Env } from '../../config';
import { rawConfigServiceMock } from '../../config/raw_config_service.mock';
import { getEnvOptions } from '../../config/__mocks__/env';
import { loggingServiceMock } from '../../logging/logging_service.mock';
import { PluginWrapper } from '../plugin';
import { PluginsConfig, PluginsConfigType, config } from '../plugins_config';
import { discover } from './plugins_discovery';
@ -37,6 +38,7 @@ const TEST_PLUGIN_SEARCH_PATHS = {
const TEST_EXTRA_PLUGIN_PATH = resolve(process.cwd(), 'my-extra-plugin');
const logger = loggingServiceMock.create();
beforeEach(() => {
mockReaddir.mockImplementation((path, cb) => {
if (path === TEST_PLUGIN_SEARCH_PATHS.nonEmptySrcPlugins) {
@ -182,12 +184,84 @@ test('properly iterates through plugin search locations', async () => {
'kibana.json'
)})`,
]);
expect(loggingServiceMock.collect(logger).warn).toMatchInlineSnapshot(`
Array [
Array [
"Explicit plugin paths [${TEST_EXTRA_PLUGIN_PATH}] are only supported in development. Relative imports will not work in production.",
],
]
`);
});
test('logs a warning about --plugin-path when used in development', async () => {
mockPackage.raw = {
branch: 'master',
version: '1.2.3',
build: {
distributable: true,
number: 1,
sha: '',
},
};
const env = Env.createDefault(
getEnvOptions({
cliArgs: { dev: false, envName: 'development' },
})
);
const configService = new ConfigService(
rawConfigServiceMock.create({ rawConfig: { plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } } }),
env,
logger
);
await configService.setSchema(config.path, config.schema);
const rawConfig = await configService
.atPath<PluginsConfigType>('plugins')
.pipe(first())
.toPromise();
discover(new PluginsConfig(rawConfig, env), {
coreId: Symbol(),
configService,
env,
logger,
});
expect(loggingServiceMock.collect(logger).warn).toEqual([
[
`Explicit plugin paths [${TEST_EXTRA_PLUGIN_PATH}] should only be used in development. Relative imports may not work properly in production.`,
],
]);
});
test('does not log a warning about --plugin-path when used in production', async () => {
mockPackage.raw = {
branch: 'master',
version: '1.2.3',
build: {
distributable: true,
number: 1,
sha: '',
},
};
const env = Env.createDefault(
getEnvOptions({
cliArgs: { dev: false, envName: 'production' },
})
);
const configService = new ConfigService(
rawConfigServiceMock.create({ rawConfig: { plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } } }),
env,
logger
);
await configService.setSchema(config.path, config.schema);
const rawConfig = await configService
.atPath<PluginsConfigType>('plugins')
.pipe(first())
.toPromise();
discover(new PluginsConfig(rawConfig, env), {
coreId: Symbol(),
configService,
env,
logger,
});
expect(loggingServiceMock.collect(logger).warn).toEqual([]);
});

View file

@ -46,9 +46,9 @@ export function discover(config: PluginsConfig, coreContext: CoreContext) {
const log = coreContext.logger.get('plugins-discovery');
log.debug('Discovering plugins...');
if (config.additionalPluginPaths.length) {
if (config.additionalPluginPaths.length && coreContext.env.mode.dev) {
log.warn(
`Explicit plugin paths [${config.additionalPluginPaths}] are only supported in development. Relative imports will not work in production.`
`Explicit plugin paths [${config.additionalPluginPaths}] should only be used in development. Relative imports may not work properly in production.`
);
}

View file

@ -0,0 +1,44 @@
/*
* 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 { PluginsConfig, PluginsConfigType } from './plugins_config';
import { Env } from '../config';
import { getEnvOptions } from '../config/__mocks__/env';
describe('PluginsConfig', () => {
it('retrieves additionalPluginPaths from config.paths when in production mode', () => {
const env = Env.createDefault(getEnvOptions({ cliArgs: { dev: false } }));
const rawConfig: PluginsConfigType = {
initialize: true,
paths: ['some-path', 'another-path'],
};
const config = new PluginsConfig(rawConfig, env);
expect(config.additionalPluginPaths).toEqual(['some-path', 'another-path']);
});
it('retrieves additionalPluginPaths from config.paths when in development mode', () => {
const env = Env.createDefault(getEnvOptions({ cliArgs: { dev: true } }));
const rawConfig: PluginsConfigType = {
initialize: true,
paths: ['some-path', 'another-path'],
};
const config = new PluginsConfig(rawConfig, env);
expect(config.additionalPluginPaths).toEqual(['some-path', 'another-path']);
});
});

View file

@ -29,7 +29,6 @@ export const config = {
/**
* Defines an array of directories where another plugin should be loaded from.
* Should only be used in a development environment.
*/
paths: schema.arrayOf(schema.string(), { defaultValue: [] }),
}),
@ -55,7 +54,6 @@ export class PluginsConfig {
constructor(rawConfig: PluginsConfigType, env: Env) {
this.initialize = rawConfig.initialize;
this.pluginSearchPaths = env.pluginSearchPaths;
// Only allow custom plugin paths in dev.
this.additionalPluginPaths = env.mode.dev ? rawConfig.paths : [];
this.additionalPluginPaths = rawConfig.paths;
}
}

View file

@ -18,6 +18,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -39,7 +40,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -89,6 +89,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -110,7 +111,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -160,6 +160,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -181,7 +182,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -235,6 +235,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -256,7 +257,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/translations/en.json",
@ -306,6 +306,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -327,7 +328,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -377,6 +377,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -398,7 +399,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -448,6 +448,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -469,7 +470,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/translations/en.json",
@ -519,6 +519,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -540,7 +541,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -592,6 +592,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -613,7 +614,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",
@ -663,6 +663,7 @@ Object {
"oss": false,
"quiet": false,
"repl": false,
"runExamples": false,
"silent": false,
"watch": false,
},
@ -684,7 +685,6 @@ Object {
"version": Any<String>,
},
"pluginSearchPaths": Any<Array>,
"staticFilesDir": Any<String>,
},
"i18n": Object {
"translationsUrl": "/mock-server-basepath/translations/en.json",

View file

@ -41,7 +41,6 @@ const INJECTED_METADATA = {
version: expect.any(String),
},
pluginSearchPaths: expect.any(Array),
staticFilesDir: expect.any(String),
},
legacyMetadata: {
branch: expect.any(String),
@ -50,6 +49,7 @@ const INJECTED_METADATA = {
version: expect.any(String),
},
};
const { createKibanaRequest, createRawRequest } = httpServerMock;
const legacyApp = { getId: () => 'legacy' };

View file

@ -76,6 +76,7 @@ export function createRootWithSettings(
repl: false,
basePath: false,
optimize: false,
runExamples: false,
oss: true,
...cliArgs,
},