diff --git a/src/cli/serve/serve.js b/src/cli/serve/serve.js index 976eac7f95da..6b13d0dc32d3 100644 --- a/src/cli/serve/serve.js +++ b/src/cli/serve/serve.js @@ -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 diff --git a/src/core/server/config/__mocks__/env.ts b/src/core/server/config/__mocks__/env.ts index 644b499ff56d..80cfab81fb55 100644 --- a/src/core/server/config/__mocks__/env.ts +++ b/src/core/server/config/__mocks__/env.ts @@ -38,6 +38,7 @@ export function getEnvOptions(options: DeepPartial = {}): EnvOptions basePath: false, optimize: false, oss: false, + runExamples: false, ...(options.cliArgs || {}), }, isDevClusterMaster: diff --git a/src/core/server/config/__snapshots__/env.test.ts.snap b/src/core/server/config/__snapshots__/env.test.ts.snap index 1f4661283de6..c36fdd8110c5 100644 --- a/src/core/server/config/__snapshots__/env.test.ts.snap +++ b/src/core/server/config/__snapshots__/env.test.ts.snap @@ -12,6 +12,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -56,6 +57,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -99,6 +101,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -142,6 +145,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -185,6 +189,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, @@ -228,6 +233,7 @@ Env { "oss": false, "quiet": false, "repl": false, + "runExamples": false, "silent": false, "watch": false, }, diff --git a/src/core/server/config/env.test.ts b/src/core/server/config/env.test.ts index 5812fa93cf18..c244012e3446 100644 --- a/src/core/server/config/env.test.ts +++ b/src/core/server/config/env.test.ts @@ -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'); +}); diff --git a/src/core/server/config/env.ts b/src/core/server/config/env.ts index 460773d89db8..369d82518923 100644 --- a/src/core/server/config/env.ts +++ b/src/core/server/config/env.ts @@ -43,6 +43,7 @@ export interface CliArgs { optimize: boolean; open: boolean; oss: boolean; + runExamples: boolean; } export class Env { @@ -104,10 +105,11 @@ export class Env { 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); diff --git a/src/core/server/plugins/discovery/plugins_discovery.test.ts b/src/core/server/plugins/discovery/plugins_discovery.test.ts index bf55fc7caae4..9cdfdbc6c74d 100644 --- a/src/core/server/plugins/discovery/plugins_discovery.test.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.test.ts @@ -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) { @@ -183,11 +185,47 @@ test('properly iterates through plugin search locations', async () => { )})`, ]); - 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.", - ], -] -`); + expect(loggingServiceMock.collect(logger).warn).toEqual([]); +}); + +test('logs a warning about --plugin-paths 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('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.`, + ], + ]); }); diff --git a/src/core/server/plugins/discovery/plugins_discovery.ts b/src/core/server/plugins/discovery/plugins_discovery.ts index 521d02e487df..f9d1a6c09d12 100644 --- a/src/core/server/plugins/discovery/plugins_discovery.ts +++ b/src/core/server/plugins/discovery/plugins_discovery.ts @@ -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.prod) { 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.` ); } diff --git a/src/core/server/plugins/plugins_config.test.ts b/src/core/server/plugins/plugins_config.test.ts new file mode 100644 index 000000000000..180d6093e040 --- /dev/null +++ b/src/core/server/plugins/plugins_config.test.ts @@ -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']); + }); +}); diff --git a/src/core/server/plugins/plugins_config.ts b/src/core/server/plugins/plugins_config.ts index 93ae229737fa..8ebcc92672dd 100644 --- a/src/core/server/plugins/plugins_config.ts +++ b/src/core/server/plugins/plugins_config.ts @@ -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; } } diff --git a/src/test_utils/kbn_server.ts b/src/test_utils/kbn_server.ts index 40700e05bcde..43c6b4378ed2 100644 --- a/src/test_utils/kbn_server.ts +++ b/src/test_utils/kbn_server.ts @@ -76,6 +76,7 @@ export function createRootWithSettings( repl: false, basePath: false, optimize: false, + runExamples: false, oss: true, ...cliArgs, },