Skip to content
8 changes: 8 additions & 0 deletions src/server/config/__tests__/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,14 @@ describe('lib/config/config', function () {
expect(config.get('myTest.test')).to.be(true);
});

it('should allow you to extend the schema with a prefix', function () {
var newSchema = Joi.object({ test: Joi.boolean().default(true) }).default();
config.extendSchema('prefix.myTest', newSchema);
expect(config.get('prefix')).to.eql({ myTest: { test: true }});
expect(config.get('prefix.myTest')).to.eql({ test: true });
expect(config.get('prefix.myTest.test')).to.be(true);
});

it('should NOT allow you to extend the schema if somethign else is there', function () {
var newSchema = Joi.object({ test: Joi.boolean().default(true) }).default();
var run = function () {
Expand Down
67 changes: 53 additions & 14 deletions src/server/config/config.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,56 @@
import Promise from 'bluebird';
import Joi from 'joi';
import _ from 'lodash';
import toPath from 'lodash/internal/toPath';
import override from './override';
import createDefaultSchema from './schema';
import pkg from '../../utils/package_json';
import clone from './deep_clone_with_buffers';
import { zipObject } from 'lodash';

const schema = Symbol('Joi Schema');
const schemaKeys = Symbol('Schema Extensions');
const schemaExts = Symbol('Schema Extensions');
const vals = Symbol('config values');
const pendingSets = Symbol('Pending Settings');

function unset(object, rawPath) {
const path = toPath(rawPath);

switch (path.length) {
case 0:
return;

case 1:
delete object[rawPath];
break;

default:
const leaf = path.pop();
const parentPath = path.slice();
const parent = _.get(object, parentPath);
delete parent[leaf];
if (!_.size(parent)) {
unset(object, parentPath);
}
break;
}
}

module.exports = class Config {
static withDefaultSchema(settings = {}) {
return new Config(createDefaultSchema(), settings);
}

constructor(initialSchema, initialSettings) {
this[schemaKeys] = new Map();

this[schemaExts] = Object.create(null);
this[vals] = Object.create(null);
this[pendingSets] = new Map(_.pairs(clone(initialSettings || {})));
this[pendingSets] = _.merge(Object.create(null), initialSettings || {});

if (initialSchema) this.extendSchema(initialSchema);
}

getPendingSets() {
return this[pendingSets];
return new Map(_.pairs(this[pendingSets]));
}

extendSchema(key, extension) {
Expand All @@ -41,26 +64,27 @@ module.exports = class Config {
throw new Error(`Config schema already has key: ${key}`);
}

this[schemaKeys].set(key, extension);
_.set(this[schemaExts], key, extension);
this[schema] = null;

let initialVals = this[pendingSets].get(key);
let initialVals = _.get(this[pendingSets], key);
if (initialVals) {
this.set(key, initialVals);
this[pendingSets].delete(key);
const path = toPath(key);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path isn't used here

unset(this[pendingSets], key);
} else {
this._commit(this[vals]);
}
}

removeSchema(key) {
if (!this[schemaKeys].has(key)) {
if (!_.has(this[schemaExts], key)) {
throw new TypeError(`Unknown schema key: ${key}`);
}

this[schema] = null;
this[schemaKeys].delete(key);
this[pendingSets].delete(key);
unset(this[schemaExts], key);
unset(this[pendingSets], key);
delete this[vals][key];
}

Expand Down Expand Up @@ -138,7 +162,7 @@ module.exports = class Config {
// Catch the partial paths
if (path.join('.') === key) return true;
// Only go deep on inner objects with children
if (schema._inner.children.length) {
if (_.size(schema._inner.children)) {
for (let i = 0; i < schema._inner.children.length; i++) {
let child = schema._inner.children[i];
// If the child is an object recurse through it's children and return
Expand All @@ -163,8 +187,23 @@ module.exports = class Config {

getSchema() {
if (!this[schema]) {
let objKeys = zipObject([...this[schemaKeys]]);
this[schema] = Joi.object().keys(objKeys).default();
this[schema] = (function convertToSchema(children) {
let objKeys = zipObject([...children]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is doing what you think it is. Key validation is added per key here.

Since children is an object I'm pretty sure that [...children] produces [], which means that zipObject() produces {} and there really isn't any reason for this to happen at all.

If anything we should probably remove the call to .keys() on the next line, since it's not doing anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the tests checks that Config throws when not given a schema but a value is set, which seems correct as it's delegating to Joi, and Joi should be making sure you don't set a value that's not part of the schema.

You're right that this code is wrong, but setting the .keys on the Joi objects is still required to get that validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After tinkering with this some more, it seems that Joi.object().keys().default() will not validate the schema as we expect, but simply passing an empty array to keys, ala Joi.object().keys([]).default() will.

let schema = Joi.object().keys(objKeys).default();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing objKeys here is unnecessary. Joi is itterating the own properties of objKeys and adding them as keys, but since it's an array it has no own properties and does nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I read the Joi docs wrong before.

The call to .keys() is required, otherwise the validation fails (and the tests do as a result), but it just needs to be given an empty object, since the actual keys are appended after the fact.


for (const key of Object.keys(children)) {
const child = children[key];
const childSchema = _.isPlainObject(child) ? convertToSchema(child) : child;

if (!childSchema || !childSchema.isJoi) {
throw new TypeError('Unable to convert configuration definition value to Joi schema: ' + childSchema);
}

schema = schema.keys({ [key]: childSchema });
}

return schema;
}(this[schemaExts]));
}

return this[schema];
Expand Down
14 changes: 9 additions & 5 deletions src/server/plugins/plugin.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import _ from 'lodash';
import toPath from 'lodash/internal/toPath';
import Joi from 'joi';
import { attempt, fromNode } from 'bluebird';
import { basename, resolve } from 'path';
Expand Down Expand Up @@ -35,6 +36,8 @@ const defaultConfigSchema = Joi.object({
* @param {String} [opts.version=pkg.version] - the version of this plugin
* @param {Function} [opts.init] - A function that will be called to initialize
* this plugin at the appropriate time.
* @param {Function} [opts.configPrefix=this.id] - The prefix to use for configuration
* values in the main configuration service
* @param {Function} [opts.config] - A function that produces a configuration
* schema using Joi, which is passed as its
* first argument.
Expand All @@ -55,6 +58,7 @@ module.exports = class Plugin {
this.requiredIds = opts.require || [];
this.version = opts.version || pkg.version;
this.externalInit = opts.init || _.noop;
this.configPrefix = opts.configPrefix || this.id;
this.getConfigSchema = opts.config || _.noop;
this.init = _.once(this.init);

Expand Down Expand Up @@ -83,18 +87,18 @@ module.exports = class Plugin {
async readConfig() {
let schema = await this.getConfigSchema(Joi);
let { config } = this.kbnServer;
config.extendSchema(this.id, schema || defaultConfigSchema);
config.extendSchema(this.configPrefix, schema || defaultConfigSchema);

if (config.get([this.id, 'enabled'])) {
if (config.get([...toPath(this.configPrefix), 'enabled'])) {
return true;
} else {
config.removeSchema(this.id);
config.removeSchema(this.configPrefix);
return false;
}
}

async init() {
let { id, version, kbnServer } = this;
let { id, version, kbnServer, configPrefix } = this;
let { config } = kbnServer;

// setup the hapi register function and get on with it
Expand Down Expand Up @@ -127,7 +131,7 @@ module.exports = class Plugin {
await fromNode(cb => {
kbnServer.server.register({
register: register,
options: config.has(id) ? config.get(id) : null
options: config.has(configPrefix) ? config.get(configPrefix) : null
}, cb);
});

Expand Down