From d8cffe4b9bc0c1e9a52c070e60bc456fa3d823a5 Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 4 Jan 2017 11:05:48 -0500 Subject: [PATCH] added configuration validation + tests --- app.js | 12 +- package.json | 1 + src/configValidation.js | 44 +++ test/unit/app.js | 35 +++ test/unit/run.js | 2 + test/unit/src/configValidation.js | 434 ++++++++++++++++++++++++++++++ 6 files changed, 523 insertions(+), 5 deletions(-) create mode 100644 src/configValidation.js create mode 100644 test/unit/app.js create mode 100644 test/unit/src/configValidation.js diff --git a/app.js b/app.js index 25573573..2dabb3c9 100644 --- a/app.js +++ b/app.js @@ -1,11 +1,13 @@ var app = require('express')(); -var peliasConfig = require( 'pelias-config' ).generate().api; +var peliasConfig = require( 'pelias-config' ).generate(); +// validate the configuration before attempting to load the app +require('./src/configValidation').validate(peliasConfig); -if( peliasConfig.accessLog ){ - app.use( require( './middleware/access_log' ).createAccessLogger( peliasConfig.accessLog ) ); +if( peliasConfig.api.accessLog ){ + app.use( require( './middleware/access_log' ).createAccessLogger( peliasConfig.api.accessLog ) ); } /** ----------------------- pre-processing-middleware ----------------------- **/ @@ -18,10 +20,10 @@ app.use( require('./middleware/jsonp') ); /** ----------------------- routes ----------------------- **/ var legacy = require('./routes/legacy'); -legacy.addRoutes(app, peliasConfig); +legacy.addRoutes(app, peliasConfig.api); var v1 = require('./routes/v1'); -v1.addRoutes(app, peliasConfig); +v1.addRoutes(app, peliasConfig.api); /** ----------------------- error middleware ----------------------- **/ diff --git a/package.json b/package.json index 9059f0a9..08549887 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "geolib": "^2.0.18", "geopipes-elasticsearch-backend": "^0.2.0", "iso3166-1": "^0.2.3", + "joi": "^10.1.0", "lodash": "^4.5.0", "markdown": "0.5.0", "morgan": "1.7.0", diff --git a/src/configValidation.js b/src/configValidation.js new file mode 100644 index 00000000..621f883b --- /dev/null +++ b/src/configValidation.js @@ -0,0 +1,44 @@ +'use strict'; + +const Joi = require('joi'); + +// Schema Configuration +// required: +// * api.version (string) +// * api.indexName (string) +// * api.host (string) +// * esclient (object - positive integer requestTimeout) +// +// optional: +// * api.accessLog (string) +// * api.relativeScores (boolean) +// * api.legacyUrl (string) +// * api.localization (flipNumberAndStreetCountries is array of 3 character strings) +const schema = Joi.object().keys({ + api: Joi.object().keys({ + version: Joi.string(), + indexName: Joi.string(), + host: Joi.string(), + legacyUrl: Joi.string(), + accessLog: Joi.string(), + relativeScores: Joi.boolean(), + localization: Joi.object().keys({ + flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/)) + }).unknown(false) + + }).requiredKeys('version', 'indexName', 'host').unknown(true), + esclient: Joi.object().keys({ + requestTimeout: Joi.number().integer().min(0) + }).unknown(true) +}).requiredKeys('api', 'esclient').unknown(true); + +module.exports = { + validate: function validate(config) { + Joi.validate(config, schema, (err) => { + if (err) { + throw new Error(err.details[0].message); + } + }); + } + +}; diff --git a/test/unit/app.js b/test/unit/app.js new file mode 100644 index 00000000..58ac8cdb --- /dev/null +++ b/test/unit/app.js @@ -0,0 +1,35 @@ +'use strict'; + +const proxyquire = require('proxyquire').noCallThru(); + +module.exports.tests = {}; + +module.exports.tests.invalid_configuration = function(test, common) { + test('configuration validation throwing error should rethrow', function(t) { + t.throws(function() { + proxyquire('../../app', { + './src/configValidation': { + validate: () => { + throw Error('config is not valid'); + } + } + }); + + }, /config is not valid/); + + t.end(); + + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('app: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; diff --git a/test/unit/run.js b/test/unit/run.js index d21890e7..017492b5 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -9,6 +9,7 @@ var common = { }; var tests = [ + require('./app'), require('./controller/index'), require('./controller/place'), require('./controller/search'), @@ -61,6 +62,7 @@ var tests = [ require('./sanitizer/_categories'), require('./sanitizer/nearby'), require('./src/backend'), + require('./src/configValidation'), require('./sanitizer/autocomplete'), require('./sanitizer/structured_geocoding'), require('./sanitizer/place'), diff --git a/test/unit/src/configValidation.js b/test/unit/src/configValidation.js new file mode 100644 index 00000000..91b34312 --- /dev/null +++ b/test/unit/src/configValidation.js @@ -0,0 +1,434 @@ +'use strict'; + +const configValidation = require('../../../src/configValidation'); + +module.exports.tests = {}; + +module.exports.tests.completely_valid = function(test, common) { + test('all valid configuration elements should not throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + legacyUrl: 'legacyUrl value', + accessLog: 'accessLog value', + relativeScores: true, + localization: { + flipNumberAndStreetCountries: ['ABC', 'DEF'] + } + }, + esclient: { + requestTimeout: 17 + } + }; + + t.doesNotThrow(function() { + configValidation.validate(config); + }); + + t.end(); + + }); + + test('basic valid configurtaion should not throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: 17 + } + }; + + t.doesNotThrow(function() { + configValidation.validate(config); + }); + + t.end(); + + }); + +}; + +module.exports.tests.api_validation = function(test, common) { + test('config without api should throw error', function(t) { + var config = { + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"api" is required/, 'api should exist'); + t.end(); + + }); + + test('config without unknown field in api should not throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + unknown: 'unknown value' + }, + esclient: {} + }; + + t.doesNotThrow(function() { + configValidation.validate(config); + }, 'unknown properties should be allowed'); + t.end(); + + }); + + test('non-string api.version should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: value, + indexName: 'index name value', + host: 'host value' + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"version" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.indexName should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: value, + host: 'host value' + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"indexName" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.host should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"host" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.legacyUrl should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + legacyUrl: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"legacyUrl" must be a string/); + }); + + t.end(); + + }); + + test('non-string api.accessLog should throw error', function(t) { + [null, 17, {}, [], true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + accessLog: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"accessLog" must be a string/); + }); + + t.end(); + + }); + + test('non-boolean api.relativeScores should throw error', function(t) { + [null, 17, {}, [], 'string'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + relativeScores: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"relativeScores" must be a boolean/); + }); + + t.end(); + + }); + + test('non-object api.localization should throw error', function(t) { + [null, 17, false, [], 'string'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: value + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"localization" must be an object/); + }); + + t.end(); + + }); + + test('unknown properties in api.localization should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + unknown_property: 'value' + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"unknown_property" is not allowed/); + + t.end(); + + }); + + test('non-array api.localization.flipNumberAndStreetCountries should throw error', function(t) { + [null, 17, {}, false, 'string'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + flipNumberAndStreetCountries: value + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"flipNumberAndStreetCountries" must be an array/); + }); + + t.end(); + + }); + + test('non-string api.localization.flipNumberAndStreetCountries elements should throw error', function(t) { + [null, 17, {}, false, []].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + flipNumberAndStreetCountries: [value] + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /"0" must be a string/); + }); + + t.end(); + + }); + + test('non-3-char api.localization.flipNumberAndStreetCountries elements should throw error', function(t) { + ['AB', 'ABCD'].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value', + localization: { + flipNumberAndStreetCountries: [value] + } + }, + esclient: {} + }; + + t.throws(function() { + configValidation.validate(config); + }, /fails to match the required pattern/); + }); + + t.end(); + + }); + +}; + +module.exports.tests.esclient_validation = function(test, common) { + test('config without esclient should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"esclient" is required/, 'esclient should exist'); + t.end(); + + }); + + test('config with non-object esclient should throw error', function(t) { + [null, 17, [], 'string', true].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: value + }; + + t.throws(function() { + configValidation.validate(config); + }, /"esclient" must be an object/, 'esclient should be an object'); + + }); + + t.end(); + + }); + + test('config with non-integer esclient.requestTimeout should throw error', function(t) { + [null, 'string', {}, [], false].forEach((value) => { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: value + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"requestTimeout" must be a number/, 'esclient.requestTimeout should be a number'); + }); + + t.end(); + + }); + + test('config with non-integer esclient.requestTimeout should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: 17.3 + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"requestTimeout" must be an integer/, 'esclient.requestTimeout should be an integer'); + + t.end(); + + }); + + test('config with negative esclient.requestTimeout should throw error', function(t) { + var config = { + api: { + version: 'version value', + indexName: 'index name value', + host: 'host value' + }, + esclient: { + requestTimeout: -1 + } + }; + + t.throws(function() { + configValidation.validate(config); + }, /"requestTimeout" must be larger than or equal to 0/, 'esclient.requestTimeout must be positive'); + + t.end(); + + }); + +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('configValidation: ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +};