diff --git a/src/cli/serve/__tests__/fixtures/invalid_config.yml b/src/cli/serve/__tests__/fixtures/invalid_config.yml new file mode 100644 index 000000000000..df9ea641cd3f --- /dev/null +++ b/src/cli/serve/__tests__/fixtures/invalid_config.yml @@ -0,0 +1,13 @@ +unknown: + key: 1 + +other: + unknown.key: 2 + third: 3 + +some.flat.key: 4 + +some.array: + - 1 + - 2 + - 3 diff --git a/src/cli/serve/__tests__/invalid_config.js b/src/cli/serve/__tests__/invalid_config.js new file mode 100644 index 000000000000..72c5ca1428a2 --- /dev/null +++ b/src/cli/serve/__tests__/invalid_config.js @@ -0,0 +1,39 @@ +import { spawnSync } from 'child_process'; +import { resolve } from 'path'; + +import expect from 'expect.js'; + +const ROOT_DIR = resolve(__dirname, '../../../../'); +const INVALID_CONFIG_PATH = resolve(__dirname, 'fixtures/invalid_config.yml'); + +describe('cli invalid config support', function () { + this.timeout(20 * 1000); + this.slow(10 * 1000); + + it('exits with statusCode 64 and logs a single line when config is invalid', function () { + const { error, status, stdout } = spawnSync(process.execPath, [ + 'src/cli', + '--config', INVALID_CONFIG_PATH + ], { + cwd: ROOT_DIR + }); + + const logLines = stdout.toString('utf8') + .split('\n') + .filter(Boolean) + .map(JSON.parse); + + expect(error).to.be(undefined); + expect(status).to.be(64); + + expect(logLines).to.have.length(1); + expect(logLines[0]).to.have.property('tags') + .eql(['fatal']); + expect(logLines[0]).to.have.property('message') + .contain('"unknown.key"') + .contain('"other.unknown.key"') + .contain('"other.third"') + .contain('"some.flat.key"') + .contain('"some.array"'); + }); +}); diff --git a/src/cli/serve/serve.js b/src/cli/serve/serve.js index dacd0002eb9b..c20bfbe6879a 100644 --- a/src/cli/serve/serve.js +++ b/src/cli/serve/serve.js @@ -147,18 +147,27 @@ export default function (program) { try { kbnServer = new KbnServer(settings); await kbnServer.ready(); - } - catch (err) { + } catch (error) { const { server } = kbnServer; - if (err.code === 'EADDRINUSE') { - logFatal(`Port ${err.port} is already in use. Another instance of Kibana may be running!`, server); - } else { - logFatal(err, server); + switch (error.code) { + case 'EADDRINUSE': + logFatal(`Port ${error.port} is already in use. Another instance of Kibana may be running!`, server); + break; + + case 'InvalidConfig': + logFatal(error.message, server); + break; + + default: + logFatal(error, server); + break; } kbnServer.close(); - process.exit(1); // eslint-disable-line no-process-exit + const exitCode = error.processExitCode == null ? 1 : error.processExitCode; + // eslint-disable-next-line no-process-exit + process.exit(exitCode); } process.on('SIGHUP', function reloadConfig() { @@ -175,6 +184,7 @@ export default function (program) { function logFatal(message, server) { if (server) { server.log(['fatal'], message); + } else { + console.error('FATAL', message); } - console.error('FATAL', message); } diff --git a/src/core_plugins/elasticsearch/lib/__tests__/routes.js b/src/core_plugins/elasticsearch/lib/__tests__/routes.js index 84be2f702395..4f88d528a207 100644 --- a/src/core_plugins/elasticsearch/lib/__tests__/routes.js +++ b/src/core_plugins/elasticsearch/lib/__tests__/routes.js @@ -1,7 +1,6 @@ import { format } from 'util'; import * as kbnTestServer from '../../../../test_utils/kbn_server'; -import { fromRoot } from '../../../../utils'; describe('plugins/elasticsearch', function () { describe('routes', function () { @@ -13,13 +12,7 @@ describe('plugins/elasticsearch', function () { // context, not to the parent context. this.timeout(60000); - kbnServer = kbnTestServer.createServer({ - plugins: { - scanDirs: [ - fromRoot('src/core_plugins') - ] - }, - }); + kbnServer = kbnTestServer.createServerWithCorePlugins(); await kbnServer.ready(); await kbnServer.server.plugins.elasticsearch.waitUntilReady(); diff --git a/src/core_plugins/kibana/server/lib/__tests__/manage_uuid.js b/src/core_plugins/kibana/server/lib/__tests__/manage_uuid.js index 10c5ccc4ce1e..216292c6c4de 100644 --- a/src/core_plugins/kibana/server/lib/__tests__/manage_uuid.js +++ b/src/core_plugins/kibana/server/lib/__tests__/manage_uuid.js @@ -1,7 +1,6 @@ import expect from 'expect.js'; import sinon from 'sinon'; import * as kbnTestServer from '../../../../../test_utils/kbn_server.js'; -import { fromRoot } from '../../../../../utils'; import manageUuid from '../manage_uuid'; describe('core_plugins/kibana/server/lib', function () { @@ -13,13 +12,7 @@ describe('core_plugins/kibana/server/lib', function () { before(async function () { this.timeout(60000); // sometimes waiting for server takes longer than 10 - kbnServer = kbnTestServer.createServer({ - plugins: { - scanDirs: [ - fromRoot('src/core_plugins') - ] - } - }); + kbnServer = kbnTestServer.createServerWithCorePlugins(); await kbnServer.ready(); }); diff --git a/src/functional_test_runner/__tests__/lib/kibana.js b/src/functional_test_runner/__tests__/lib/kibana.js index 8004f2f2035d..478ce9df4ffc 100644 --- a/src/functional_test_runner/__tests__/lib/kibana.js +++ b/src/functional_test_runner/__tests__/lib/kibana.js @@ -1,20 +1,12 @@ -import { resolve } from 'path'; - -import { createServer } from '../../../test_utils/kbn_server'; +import { createServerWithCorePlugins } from '../../../test_utils/kbn_server'; export async function startupKibana({ port, esUrl }) { - const server = createServer({ + const server = createServerWithCorePlugins({ server: { port, autoListen: true, }, - plugins: { - scanDirs: [ - resolve(__dirname, '../../../core_plugins') - ], - }, - elasticsearch: { url: esUrl } diff --git a/src/server/config/__tests__/complete.js b/src/server/config/__tests__/complete.js index 668c45587e6d..b4e5c784fba3 100644 --- a/src/server/config/__tests__/complete.js +++ b/src/server/config/__tests__/complete.js @@ -1,105 +1,200 @@ -import complete from '../complete'; +import completeMixin from '../complete'; import expect from 'expect.js'; -import { noop } from 'lodash'; import sinon from 'sinon'; -describe('server config complete', function () { - it(`should call server.log when there's an unused setting`, function () { - const kbnServer = { - settings: { - unused: true - } - }; +/* eslint-disable import/no-duplicates */ +import * as transformDeprecationsNS from '../transform_deprecations'; +import { transformDeprecations } from '../transform_deprecations'; +/* eslint-enable import/no-duplicates */ + +describe('server/config completeMixin()', function () { + const sandbox = sinon.sandbox.create(); + afterEach(() => sandbox.restore()); + + const setup = (options = {}) => { + const { + settings = {}, + configValues = {} + } = options; const server = { - decorate: noop, - log: sinon.spy() + decorate: sinon.stub() }; const config = { - get: sinon.stub().returns({ - used: true - }) + get: sinon.stub().returns(configValues) }; - complete(kbnServer, server, config); + const kbnServer = { + settings, + server, + config + }; - expect(server.log.calledOnce).to.be(true); + const callCompleteMixin = () => { + completeMixin(kbnServer, server, config); + }; + + return { config, callCompleteMixin, server }; + }; + + describe('server decoration', () => { + it('adds a config() function to the server', () => { + const { config, callCompleteMixin, server } = setup({ + settings: {}, + configValues: {} + }); + + callCompleteMixin(); + sinon.assert.calledOnce(server.decorate); + sinon.assert.calledWith(server.decorate, 'server', 'config', sinon.match.func); + expect(server.decorate.firstCall.args[2]()).to.be(config); + }); }); - it(`shouldn't call server.log when there isn't an unused setting`, function () { - const kbnServer = { - settings: { - used: true - } - }; + describe('all settings used', () => { + it('should not throw', function () { + const { callCompleteMixin } = setup({ + settings: { + used: true + }, + configValues: { + used: true + }, + }); - const server = { - decorate: noop, - log: sinon.spy() - }; + callCompleteMixin(); + }); - const config = { - get: sinon.stub().returns({ - used: true - }) - }; + describe('more config values than settings', () => { + it('should not throw', function () { + const { callCompleteMixin } = setup({ + settings: { + used: true + }, + configValues: { + used: true, + foo: 'bar' + } + }); - complete(kbnServer, server, config); - - expect(server.log.called).to.be(false); + callCompleteMixin(); + }); + }); }); - it(`shouldn't call server.log when there are more config values than settings`, function () { - const kbnServer = { - settings: { - used: true - } - }; - - const server = { - decorate: noop, - log: sinon.spy() - }; - - const config = { - get: sinon.stub().returns({ - used: true, - foo: 'bar' - }) - }; - - complete(kbnServer, server, config); - expect(server.log.called).to.be(false); - }); - - it('should transform server.ssl.cert to server.ssl.certificate', function () { - const kbnServer = { - settings: { - server: { - ssl: { - cert: 'path/to/cert' + describe('env setting specified', () => { + it('should not throw', () => { + const { callCompleteMixin } = setup({ + settings: { + env: 'development' + }, + configValues: { + env: { + name: 'development' } } - } - }; + }); - const server = { - decorate: noop, - log: sinon.spy() - }; + callCompleteMixin(); + }); + }); - const config = { - get: sinon.stub().returns({ - server: { - ssl: { - certificate: 'path/to/cert' - } + describe('settings include non-default array settings', () => { + it('should not throw', () => { + const { callCompleteMixin } = setup({ + settings: { + foo: [ + 'a', + 'b' + ] + }, + configValues: { + foo: [] } - }) - }; + }); - complete(kbnServer, server, config); - expect(server.log.called).to.be(false); + callCompleteMixin(); + }); + }); + + describe('some settings unused', () => { + it('should throw an error', function () { + const { callCompleteMixin } = setup({ + settings: { + unused: true + }, + configValues: { + used: true + } + }); + + expect(callCompleteMixin).to.throwError('"unused" not applied'); + }); + + describe('error thrown', () => { + it('has correct code, processExitCode, and message', () => { + const { callCompleteMixin } = setup({ + settings: { + unused: true, + foo: 'bar', + namespace: { + with: { + sub: { + keys: true + } + } + } + } + }); + + expect(callCompleteMixin).to.throwError((error) => { + expect(error).to.have.property('code', 'InvalidConfig'); + expect(error).to.have.property('processExitCode', 64); + expect(error.message).to.contain('"unused", "foo", and "namespace.with.sub.keys"'); + }); + }); + }); + }); + + describe('deprecation support', () => { + it('should transform settings when determining what is unused', function () { + sandbox.spy(transformDeprecationsNS, 'transformDeprecations'); + + const settings = { + foo: 1 + }; + + const { callCompleteMixin } = setup({ + settings, + configValues: { + ...settings + } + }); + + callCompleteMixin(); + sinon.assert.calledOnce(transformDeprecations); + sinon.assert.calledWithExactly(transformDeprecations, settings); + }); + + it('should use transformed settings when considering what is used', function () { + sandbox.stub(transformDeprecationsNS, 'transformDeprecations', (settings) => { + settings.bar = settings.foo; + delete settings.foo; + return settings; + }); + + const { callCompleteMixin } = setup({ + settings: { + foo: 1 + }, + configValues: { + bar: 1 + } + }); + + callCompleteMixin(); + sinon.assert.calledOnce(transformDeprecations); + }); }); }); diff --git a/src/server/config/__tests__/flatten_with.js b/src/server/config/__tests__/flatten_with.js deleted file mode 100644 index 78864e490bc3..000000000000 --- a/src/server/config/__tests__/flatten_with.js +++ /dev/null @@ -1,27 +0,0 @@ -import flattenWith from '../flatten_with'; -import expect from 'expect.js'; - -describe('flatten_with(dot, nestedObj)', function () { - - it('should flatten object with dot', function () { - const nestedObj = { - test: { - enable: true, - hosts: ['host-01', 'host-02'], - client: { - type: 'nosql', - pool: [{ port: 5051 }, { port: 5052 }] - } - } - }; - expect(flattenWith('.', nestedObj)).to.eql({ - 'test.enable': true, - 'test.hosts': ['host-01', 'host-02'], - 'test.client.type': 'nosql', - 'test.client.pool': [{ port: 5051 }, { port: 5052 }] - }); - }); - -}); - - diff --git a/src/server/config/complete.js b/src/server/config/complete.js index 0831016cd256..17ad5f5896b0 100644 --- a/src/server/config/complete.js +++ b/src/server/config/complete.js @@ -1,8 +1,23 @@ -import { difference, keys } from 'lodash'; +import { difference } from 'lodash'; import { transformDeprecations } from './transform_deprecations'; +import { formatListAsProse, getFlattenedObject } from '../../utils'; -const getUnusedSettings = (settings, configValues) => { - return difference(keys(transformDeprecations(settings)), keys(configValues)); +const getFlattenedKeys = object => ( + Object.keys(getFlattenedObject(object)) +); + +const getUnusedConfigKeys = (settings, configValues) => { + const inputKeys = getFlattenedKeys(transformDeprecations(settings)); + const appliedKeys = getFlattenedKeys(configValues); + + if (inputKeys.includes('env')) { + // env is a special case key, see https://github.com/elastic/kibana/blob/848bf17b/src/server/config/config.js#L74 + // where it is deleted from the settings before being injected into the schema via context and + // then renamed to `env.name` https://github.com/elastic/kibana/blob/848bf17/src/server/config/schema.js#L17 + inputKeys[inputKeys.indexOf('env')] = 'env.name'; + } + + return difference(inputKeys, appliedKeys); }; export default function (kbnServer, server, config) { @@ -11,7 +26,24 @@ export default function (kbnServer, server, config) { return kbnServer.config; }); - for (const key of getUnusedSettings(kbnServer.settings, config.get())) { - server.log(['warning', 'config'], `Settings for "${key}" were not applied, check for spelling errors and ensure the plugin is loaded.`); + const unusedKeys = getUnusedConfigKeys(kbnServer.settings, config.get()) + .map(key => `"${key}"`); + + if (!unusedKeys.length) { + return; } + + const desc = unusedKeys.length === 1 + ? 'setting was' + : 'settings were'; + + const error = new Error( + `${formatListAsProse(unusedKeys)} ${desc} not applied. ` + + 'Check for spelling errors and ensure that expected ' + + 'plugins are installed and enabled.' + ); + + error.code = 'InvalidConfig'; + error.processExitCode = 64; + throw error; } diff --git a/src/server/config/flatten_with.js b/src/server/config/flatten_with.js deleted file mode 100644 index 2df9fd2f5c05..000000000000 --- a/src/server/config/flatten_with.js +++ /dev/null @@ -1,18 +0,0 @@ -import _ from 'lodash'; - -export default function (dot, nestedObj, flattenArrays) { - const stack = []; // track key stack - const flatObj = {}; - (function flattenObj(obj) { - _.keys(obj).forEach(function (key) { - stack.push(key); - if (!flattenArrays && _.isArray(obj[key])) flatObj[stack.join(dot)] = obj[key]; - else if (_.isObject(obj[key])) flattenObj(obj[key]); - else flatObj[stack.join(dot)] = obj[key]; - stack.pop(); - }); - }(nestedObj)); - return flatObj; -} - - diff --git a/src/server/config/override.js b/src/server/config/override.js index e4845d752392..4c1bf0fecfa5 100644 --- a/src/server/config/override.js +++ b/src/server/config/override.js @@ -1,11 +1,9 @@ import _ from 'lodash'; -import flattenWith from './flatten_with'; import explodeBy from './explode_by'; +import { getFlattenedObject } from '../../utils'; export default function (target, source) { - const _target = flattenWith('.', target); - const _source = flattenWith('.', source); + const _target = getFlattenedObject(target); + const _source = getFlattenedObject(source); return explodeBy('.', _.defaults(_source, _target)); } - - diff --git a/src/server/http/__tests__/index.js b/src/server/http/__tests__/index.js index 43512950a31b..89914c9ff60e 100644 --- a/src/server/http/__tests__/index.js +++ b/src/server/http/__tests__/index.js @@ -1,6 +1,5 @@ import expect from 'expect.js'; import * as kbnTestServer from '../../../test_utils/kbn_server'; -import { fromRoot } from '../../../utils'; describe('routes', function () { this.slow(10000); @@ -8,13 +7,7 @@ describe('routes', function () { let kbnServer; beforeEach(function () { - kbnServer = kbnTestServer.createServer({ - plugins: { - scanDirs: [ - fromRoot('src/core_plugins') - ] - } - }); + kbnServer = kbnTestServer.createServerWithCorePlugins(); return kbnServer.ready(); }); afterEach(function () { diff --git a/src/server/plugins/plugin.js b/src/server/plugins/plugin.js index 699e461f9b25..f2ffcff0ee35 100644 --- a/src/server/plugins/plugin.js +++ b/src/server/plugins/plugin.js @@ -117,9 +117,9 @@ export default class Plugin { const asyncRegister = async (server, options) => { this.server = server; - await Promise.all(this[extendInitFns].map(async fn => { + for (const fn of this[extendInitFns]) { await fn.call(this, server, options); - })); + } server.log(['plugins', 'debug'], { tmpl: 'Initializing plugin <%= plugin.toString() %>', diff --git a/src/test_utils/kbn_server.js b/src/test_utils/kbn_server.js index 095ae9f167db..b33b5facf94e 100644 --- a/src/test_utils/kbn_server.js +++ b/src/test_utils/kbn_server.js @@ -1,11 +1,12 @@ import url from 'url'; +import { resolve } from 'path'; import { defaultsDeep, set } from 'lodash'; import { header as basicAuthHeader } from './base_auth'; import { kibanaUser, kibanaServer } from '../../test/shield'; import { esTestServerUrlParts } from '../../test/es_test_server_url_parts'; import KbnServer from '../../src/server/kbn_server'; -const SERVER_DEFAULTS = { +const DEFAULTS_SETTINGS = { server: { autoListen: false, xsrf: { @@ -19,6 +20,14 @@ const SERVER_DEFAULTS = { optimize: { enabled: false }, +}; + +const DEFAULT_SETTINGS_WITH_CORE_PLUGINS = { + plugins: { + scanDirs: [ + resolve(__dirname, '../core_plugins'), + ], + }, elasticsearch: { url: url.format(esTestServerUrlParts), username: kibanaServer.username, @@ -26,15 +35,27 @@ const SERVER_DEFAULTS = { } }; + /** * Creates an instance of KbnServer with default configuration * tailored for unit tests * - * @param {object} params Any config overrides for this instance + * @param {Object} [settings={}] Any config overrides for this instance + * @return {KbnServer} */ -export function createServer(params = {}) { - params = defaultsDeep({}, params, SERVER_DEFAULTS); - return new KbnServer(params); +export function createServer(settings = {}) { + return new KbnServer(defaultsDeep({}, settings, DEFAULTS_SETTINGS)); +} + +/** + * Creates an instance of KbnServer, including all of the core plugins, + * with default configuration tailored for unit tests + * + * @param {Object} [settings={}] + * @return {KbnServer} + */ +export function createServerWithCorePlugins(settings = {}) { + return new KbnServer(defaultsDeep({}, settings, DEFAULT_SETTINGS_WITH_CORE_PLUGINS, DEFAULTS_SETTINGS)); } /** diff --git a/src/ui/__tests__/ui_exports.js b/src/ui/__tests__/ui_exports.js index 1f3d8e4467a4..71ed5896f28d 100644 --- a/src/ui/__tests__/ui_exports.js +++ b/src/ui/__tests__/ui_exports.js @@ -36,8 +36,8 @@ describe('UiExports', function () { kbnServer = kbnTestServer.createServer({ plugins: { paths: [ + resolve(__dirname, 'fixtures/plugin_foo'), resolve(__dirname, 'fixtures/plugin_bar'), - resolve(__dirname, 'fixtures/plugin_foo') ] }, @@ -75,7 +75,7 @@ describe('UiExports', function () { scanDirs: [], paths: [ resolve(__dirname, 'fixtures/plugin_async_foo'), - resolve(__dirname, 'fixtures/plugin_foo') + resolve(__dirname, 'fixtures/plugin_bar'), ] }, diff --git a/src/ui/ui_exports.js b/src/ui/ui_exports.js index 3808037de971..c67eeed86a9a 100644 --- a/src/ui/ui_exports.js +++ b/src/ui/ui_exports.js @@ -125,7 +125,7 @@ export default class UiExports { case 'injectDefaultVars': return (plugin, injector) => { plugin.extendInit(async (server, options) => { - _.merge(this.defaultInjectedVars, await injector.call(plugin, server, options)); + _.defaults(this.defaultInjectedVars, await injector.call(plugin, server, options)); }); }; diff --git a/src/utils/__tests__/get_flattened_object.js b/src/utils/__tests__/get_flattened_object.js new file mode 100644 index 000000000000..e51615593bff --- /dev/null +++ b/src/utils/__tests__/get_flattened_object.js @@ -0,0 +1,26 @@ +import expect from 'expect.js'; + +import { getFlattenedObject } from '../get_flattened_object'; + +describe('getFlattenedObject()', () => { + it('throws when rootValue is not an object or is an array', () => { + expect(() => getFlattenedObject(1)).to.throwError('number'); + expect(() => getFlattenedObject(Infinity)).to.throwError('number'); + expect(() => getFlattenedObject(NaN)).to.throwError('number'); + expect(() => getFlattenedObject(false)).to.throwError('boolean'); + expect(() => getFlattenedObject(null)).to.throwError('null'); + expect(() => getFlattenedObject(undefined)).to.throwError('undefined'); + expect(() => getFlattenedObject([])).to.throwError('array'); + }); + + it('flattens objects', () => { + expect(getFlattenedObject({ a: 'b' })).to.eql({ a: 'b' }); + expect(getFlattenedObject({ a: { b: 'c' } })).to.eql({ 'a.b': 'c' }); + expect(getFlattenedObject({ a: { b: 'c' }, d: { e: 'f' } })).to.eql({ 'a.b': 'c', 'd.e': 'f' }); + }); + + it('does not flatten arrays', () => { + expect(getFlattenedObject({ a: ['b'] })).to.eql({ a: ['b'] }); + expect(getFlattenedObject({ a: { b: ['c', 'd'] } })).to.eql({ 'a.b': ['c', 'd'] }); + }); +}); diff --git a/src/utils/get_flattened_object.js b/src/utils/get_flattened_object.js new file mode 100644 index 000000000000..7f5f9fe09ef3 --- /dev/null +++ b/src/utils/get_flattened_object.js @@ -0,0 +1,35 @@ + +function shouldReadKeys(value) { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Flattens a deeply nested object to a map of dot-separated + * paths pointing to all primative values **and arrays** + * from `rootValue`. + * + * example: + * getFlattenedObject({ a: { b: 1, c: [2,3] } }) + * // => { 'a.b': 1, 'a.c': [2,3] } + * + * @param {Object} rootValue + * @returns {Object} + */ +export function getFlattenedObject(rootValue) { + if (!shouldReadKeys(rootValue)) { + throw new TypeError(`Root value is not flatten-able, received ${rootValue}`); + } + + return (function flatten(acc, prefix, object) { + return Object.keys(object).reduce((acc, key) => { + const value = object[key]; + const path = prefix ? `${prefix}.${key}` : key; + + if (shouldReadKeys(value)) { + return flatten(acc, path, value); + } else { + return { ...acc, [path]: value }; + } + }, acc); + }({}, '', rootValue)); +} diff --git a/src/utils/index.js b/src/utils/index.js index 5f195647f4c9..2897654dede3 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -7,6 +7,7 @@ export { unset } from './unset'; export { encodeQueryComponent } from './encode_query_component'; export { modifyUrl } from './modify_url'; export { createToolingLog } from './tooling_log'; +export { getFlattenedObject } from './get_flattened_object'; export { getKbnTypeNames,