feat: Merge pull request #775 from pelias/add-configuration-validation

added configuration validation + tests

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 ){
+if( peliasConfig.api.accessLog ){
-  app.use( require( './middleware/access_log' ).createAccessLogger( peliasConfig.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 ----------------------- **/
 

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",

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);
+      }
+    });
+  }
+
+};

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);
+  }
+};

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'),

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);
+  }
+};