From d7e2713897f32d1a18358af0bdb91a8ef9ee69f3 Mon Sep 17 00:00:00 2001 From: mamaz Date: Sun, 9 Dec 2018 20:04:54 +0700 Subject: [PATCH 01/16] Add environment validator --- lib/env_validator.js | 27 +++++++++++++++++++ test/env_validator.spec.js | 53 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 lib/env_validator.js create mode 100644 test/env_validator.spec.js diff --git a/lib/env_validator.js b/lib/env_validator.js new file mode 100644 index 0000000..8169963 --- /dev/null +++ b/lib/env_validator.js @@ -0,0 +1,27 @@ +"use strict"; +const Config = require("./config"); +const isNil = require("lodash/isNil"); + +exports.validateEnvironment = (configuration) => { + const config = new Config(null); + const currentEnv = process.env; + if (isNil(currentEnv)) throw new Error("No environment variable installed in this system"); + + const flattenConfiguration = config._flatten(configuration); + const regex = /[${}]/g; + + for (const key of Object.keys(flattenConfiguration)) { + const value = flattenConfiguration[key]; + if (isNil(value)) { + throw new Error(`Error on Config, '${key}' is needed, but the value is ${value}`); + } + + if (regex.test(value)) { + const pureValue = value.replace(regex, ""); + const envValue = currentEnv[pureValue]; + if (!envValue || envValue === "") throw new Error(`Error on Environment, '${pureValue}' is needed, but the value is ${envValue}`); + } + } + return true; +}; + diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js new file mode 100644 index 0000000..f4ba98b --- /dev/null +++ b/test/env_validator.spec.js @@ -0,0 +1,53 @@ +"use strict"; +const assert = require("assert"); +const envValidator = require("../lib/env_validator"); + +/* eslint-env mocha */ + +describe("Name of the group", () => { + let config; + process.env.GEIST_URI = "https://example.com"; + process.env.GEIST_TOKEN = "asasaklns12io1u31oi2u3"; + + beforeEach(() => { + config = { + geist: { + type: "proxy", + uri: "${$GEIST_URI}", + version: "v1", + secret: "${$GEIST_TOKEN}" + } + }; + }); + + it("should be able to validate okay, environment from JSON config file", () => { + const result = envValidator.validateEnvironment(config); + assert.equal(result, true); + }); + + it("should be able to throw error if needed config value is not set on environment variables", () => { + config.diaenne = { + type: "proxy", + uri: "${$DIAENNE_URI}", + version: "v1" + }; + try { + envValidator.validateEnvironment(config); + } catch (e) { + assert.equal(e.message, "Error on Environment, 'DIAENNE_URI' is needed, but the value is undefined"); + } + }); + + it("should be able to throw error if needed config value is null / undefined", () => { + config.diaenne = { + type: null, + uri: "${$DIAENNE_URI}", + version: "v1" + }; + try { + envValidator.validateEnvironment(config); + } catch (e) { + assert.equal(e.message, "Error on Config, 'diaenne.type' is needed, but the value is null"); + } + }); +}); From e54596f26d5eae1309e8ef294264ca36a1ecfd21 Mon Sep 17 00:00:00 2001 From: mamaz Date: Tue, 18 Dec 2018 13:55:16 +0700 Subject: [PATCH 02/16] Change env validator to return array --- lib/env_validator.js | 21 ++++++++++++++------- test/env_validator.spec.js | 35 +++++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/lib/env_validator.js b/lib/env_validator.js index 8169963..8ebf4e9 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -2,14 +2,18 @@ const Config = require("./config"); const isNil = require("lodash/isNil"); -exports.validateEnvironment = (configuration) => { +/** + * + * + */ +exports.validateEnvironment = (environment, configuration) => { const config = new Config(null); - const currentEnv = process.env; - if (isNil(currentEnv)) throw new Error("No environment variable installed in this system"); + if (isNil(environment)) throw new Error("No environment variable installed in this system"); const flattenConfiguration = config._flatten(configuration); const regex = /[${}]/g; + const invalidValues = []; for (const key of Object.keys(flattenConfiguration)) { const value = flattenConfiguration[key]; if (isNil(value)) { @@ -17,11 +21,14 @@ exports.validateEnvironment = (configuration) => { } if (regex.test(value)) { - const pureValue = value.replace(regex, ""); - const envValue = currentEnv[pureValue]; - if (!envValue || envValue === "") throw new Error(`Error on Environment, '${pureValue}' is needed, but the value is ${envValue}`); + const pureValue = value.replace(regex, ""); // replace `${}` with "" + const envValue = environment[pureValue]; + + if (isNil(envValue) || envValue === "") { + invalidValues.push(pureValue); + } } } - return true; + return invalidValues; }; diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index f4ba98b..93d157f 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -20,34 +20,41 @@ describe("Name of the group", () => { }; }); - it("should be able to validate okay, environment from JSON config file", () => { - const result = envValidator.validateEnvironment(config); - assert.equal(result, true); + it("should return empty array if env needed is set already", () => { + const result = envValidator.validateEnvironment(process.env, config); + assert.deepStrictEqual(result, []); }); - it("should be able to throw error if needed config value is not set on environment variables", () => { + it("should be able to return not set environment variables", () => { config.diaenne = { type: "proxy", uri: "${$DIAENNE_URI}", - version: "v1" + version: "${$VERSION}" }; - try { - envValidator.validateEnvironment(config); - } catch (e) { - assert.equal(e.message, "Error on Environment, 'DIAENNE_URI' is needed, but the value is undefined"); - } + const result = envValidator.validateEnvironment(process.env, config); + assert.deepStrictEqual(result, ["DIAENNE_URI", "VERSION"]); }); - it("should be able to throw error if needed config value is null / undefined", () => { + it("should throw error if one of the variable contains null", () => { config.diaenne = { type: null, uri: "${$DIAENNE_URI}", - version: "v1" + version: "${$VERSION}" }; try { - envValidator.validateEnvironment(config); - } catch (e) { + envValidator.validateEnvironment(process.env, config); + assert.fail("Should throw error on Config"); + } catch(e) { assert.equal(e.message, "Error on Config, 'diaenne.type' is needed, but the value is null"); } }); + + it("should throw error if no environment variables is not installed in this system", () => { + try { + envValidator.validateEnvironment(null, config); + assert.fail("Should throw error on Config"); + } catch(e) { + assert.equal(e.message, "No environment variable installed in this system"); + } + }); }); From 638c4e586e79a3075ffdd30a142689f9f4366d50 Mon Sep 17 00:00:00 2001 From: mamaz Date: Wed, 26 Dec 2018 16:30:33 +0700 Subject: [PATCH 03/16] Add more tests --- index.js | 18 ++++++++++ lib/env_validator.js | 27 ++++++++++----- test/env_validator.spec.js | 21 ++++++++---- test/merapi_test.spec.js | 70 ++++++++++++++++++-------------------- 4 files changed, 85 insertions(+), 51 deletions(-) diff --git a/index.js b/index.js index fb336b5..b2c1373 100644 --- a/index.js +++ b/index.js @@ -10,6 +10,7 @@ const Injector = require("./lib/injector"); const Component = require("./lib/component"); const Logger = require("./lib/logger"); const utils = require("./lib/utils"); +const EnvValidator = require("./lib/env_validator"); function merapi(options) { return new Container(options); @@ -150,6 +151,10 @@ class Container extends Component.mixin(AsyncEmitter) { } *_initialize() { + yield this.emit("beforeValidateConfig", this); + this.validateConfig(); + yield this.emit("afterValidateConfig", this); + yield this.emit("beforeInit", this); yield this.emit("beforeConfigResolve", this); this.config.resolve(); @@ -315,6 +320,19 @@ class Container extends Component.mixin(AsyncEmitter) { } } } + + validateConfig() { + let result = []; + if (this.options.envConfig) result = EnvValidator.validateEnvironment(this.options.envConfig[this.config.env], this.options.config); + if (result.length > 0) { + result = EnvValidator.validateEnvironment(process.env, this.options.config); + if (result.length > 0) { + this.logger.log("These configurations are not set on env variables: "); + this.logger.log(result); + throw new Error("Configuration error"); + } + } + } } merapi.Container = Container; diff --git a/lib/env_validator.js b/lib/env_validator.js index 8ebf4e9..954f703 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -3,32 +3,43 @@ const Config = require("./config"); const isNil = require("lodash/isNil"); /** + * Make sure environment variables needed in configuration exists. * + * Return environment variables needed to be defined in process.env. + * + * @param {object} environment Environment variables to be validated, intended to be filled with process.env object + * @param {object} configuration Configuration used for merapi + * @param {object} delimiters Delimiters object, used to parse variables. Examples: + * formatted in { + * left: "${$" + * right: "}" + * } + * for entry ${$SOME_ENV} * */ -exports.validateEnvironment = (environment, configuration) => { +exports.validateEnvironment = (environment, configuration, delimiters = { left: "${", right: "}" }) => { const config = new Config(null); if (isNil(environment)) throw new Error("No environment variable installed in this system"); const flattenConfiguration = config._flatten(configuration); - const regex = /[${}]/g; - const invalidValues = []; + const neededValues = []; for (const key of Object.keys(flattenConfiguration)) { const value = flattenConfiguration[key]; if (isNil(value)) { throw new Error(`Error on Config, '${key}' is needed, but the value is ${value}`); } - if (regex.test(value)) { - const pureValue = value.replace(regex, ""); // replace `${}` with "" - const envValue = environment[pureValue]; + if (value.includes(delimiters.left) && + value.includes(delimiters.right)) { + const envKey = value.substring(delimiters.left.length, value.length - delimiters.right.length); + const envValue = environment[envKey]; if (isNil(envValue) || envValue === "") { - invalidValues.push(pureValue); + neededValues.push(envKey); } } } - return invalidValues; + return neededValues; }; diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index 93d157f..7b454b2 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -4,10 +4,14 @@ const envValidator = require("../lib/env_validator"); /* eslint-env mocha */ -describe("Name of the group", () => { +describe("Env validator", () => { let config; process.env.GEIST_URI = "https://example.com"; process.env.GEIST_TOKEN = "asasaklns12io1u31oi2u3"; + const delimiters = { + left: "${$", + right: "}" + }; beforeEach(() => { config = { @@ -21,18 +25,21 @@ describe("Name of the group", () => { }); it("should return empty array if env needed is set already", () => { - const result = envValidator.validateEnvironment(process.env, config); + const result = envValidator.validateEnvironment(process.env, config, delimiters); assert.deepStrictEqual(result, []); }); - it("should be able to return not set environment variables", () => { + it("should be able to return needed-to-be-set environment variables", () => { + config.geist.lala = "${$LALA}"; config.diaenne = { type: "proxy", uri: "${$DIAENNE_URI}", version: "${$VERSION}" }; - const result = envValidator.validateEnvironment(process.env, config); - assert.deepStrictEqual(result, ["DIAENNE_URI", "VERSION"]); + config.auth = "${$SECRET}"; + + const result = envValidator.validateEnvironment(process.env, config, delimiters); + assert.deepStrictEqual(result, ["LALA", "DIAENNE_URI", "VERSION", "SECRET"]); }); it("should throw error if one of the variable contains null", () => { @@ -42,7 +49,7 @@ describe("Name of the group", () => { version: "${$VERSION}" }; try { - envValidator.validateEnvironment(process.env, config); + envValidator.validateEnvironment(process.env, config, delimiters); assert.fail("Should throw error on Config"); } catch(e) { assert.equal(e.message, "Error on Config, 'diaenne.type' is needed, but the value is null"); @@ -51,7 +58,7 @@ describe("Name of the group", () => { it("should throw error if no environment variables is not installed in this system", () => { try { - envValidator.validateEnvironment(null, config); + envValidator.validateEnvironment(null, config, delimiters); assert.fail("Should throw error on Config"); } catch(e) { assert.equal(e.message, "No environment variable installed in this system"); diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index 728d06d..23a444c 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -10,12 +10,12 @@ process.env.NODE_ENV = process.NODE_ENV || "test"; /* eslint-env mocha */ describe("Merapi Test", function() { - - + + describe("Config", function() { - + let container = null; - + beforeEach(asyn(function*() { container = merapi({ delimiters: { @@ -27,37 +27,35 @@ describe("Merapi Test", function() { "myConfig": "${resolved.a}", "myEnvConfig": "${resolved.b}", "myStrEnvConfig": "${resolved.c}", - "myCrlfStrEnvConfig": "${resolved.d}", - "resolved": { - "a": 1 - } + "myCrlfStrEnvConfig": "${resolved.d}" }, - + envConfig: { test: { + "resolved.a": 1, "resolved.b": 2, "resolved.c": "test", "resolved.d": "test\r", } }, - + extConfig: { more: true } }); - + yield container.initialize(); })); - + it("can resolve config", function() { assert.notEqual(container, null); - + let myConfig = container.config.get("myConfig"); let pkg = container.config.get("package"); assert.equal(myConfig, 1); assert.equal(pkg.name, "merapi-test"); }); - + it("can resolve environment config", function() { let myEnvConfig = container.config.get("myEnvConfig"); let myStrEnvConfig = container.config.get("myStrEnvConfig"); @@ -66,26 +64,26 @@ describe("Merapi Test", function() { assert.equal(myStrEnvConfig, "test"); assert.equal(myCrlfStrEnvConfig, "test"); }); - + it("can resolve extended config", function() { assert.equal(container.config.get("more"), true); }); - + it("can resolve environment variables", function() { let ENV = container.config.get("ENV"); let env = container.config.get("env"); - + assert.equal(env, process.env.NODE_ENV); assert.equal(ENV.NODE_ENV, process.env.NODE_ENV); assert.equal(ENV.PATH, process.env.PATH); }); }); - + describe("Components", function() { - + let container = null; let obj = {}; - + beforeEach(asyn(function*() { container = merapi({ @@ -103,7 +101,7 @@ describe("Merapi Test", function() { container.register("obj", obj, true); container.alias("alias", "obj"); - + yield container.initialize(); })); @@ -131,12 +129,12 @@ describe("Merapi Test", function() { yield container.start(); assert.equal(config.default("autoloaded", false), true); })); - + it("can resolve component loader", asyn(function*() { const comTest = yield container.resolve("comTest"); assert.notEqual(comTest, null); })); - + it("can resolve component class", asyn(function*() { const comClassTest = yield container.resolve("comClassTest"); assert.notEqual(comClassTest, null); @@ -228,19 +226,19 @@ describe("Merapi Test", function() { assert.equal(warningMessage, "No main defined"); })); }); - + describe("Starter", function() { - + it("can start a main module from component loader", asyn(function*() { - + let container = merapi({ basepath: __dirname, config: { } }); - + let testVal = 0; - + container.register("mainCom", function() { return { start() { @@ -248,18 +246,18 @@ describe("Merapi Test", function() { } }; }); - + let config = yield container.resolve("config"); - + config.set("main", "mainCom"); - + assert.equal(testVal, 0); yield container.initialize(); assert.equal(testVal, 0); yield container.start(); assert.equal(testVal, 1); })); - + it("can start a main module from component class", asyn(function*() { let testVal = 0; let container = merapi({ @@ -268,21 +266,21 @@ describe("Merapi Test", function() { main: "mainCom" } }); - + container.register("mainCom", class MainComponent extends Component { *start() { yield sleep(1); testVal = 1; } }); - - + + assert.equal(testVal, 0); yield container.initialize(); assert.equal(testVal, 0); yield container.start(); assert.equal(testVal, 1); - + })); }); }); \ No newline at end of file From 5a587a2c1937e77077cad37e57d9602f879bf6d7 Mon Sep 17 00:00:00 2001 From: mamaz Date: Wed, 26 Dec 2018 17:56:30 +0700 Subject: [PATCH 04/16] Add more guard to validator --- index.js | 19 +++++++++++-------- lib/env_validator.js | 12 +++++++++--- test/env_validator.spec.js | 4 ++++ 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index b2c1373..b6b05a6 100644 --- a/index.js +++ b/index.js @@ -322,16 +322,19 @@ class Container extends Component.mixin(AsyncEmitter) { } validateConfig() { - let result = []; - if (this.options.envConfig) result = EnvValidator.validateEnvironment(this.options.envConfig[this.config.env], this.options.config); + const combinedEnv = Object.assign( + {}, + this.options.envConfig && this.options.envConfig[this.config.env], + this.options.extConfig, + process.env + ); + const result = EnvValidator.validateEnvironment(combinedEnv, this.options.config); if (result.length > 0) { - result = EnvValidator.validateEnvironment(process.env, this.options.config); - if (result.length > 0) { - this.logger.log("These configurations are not set on env variables: "); - this.logger.log(result); - throw new Error("Configuration error"); - } + this.logger.log("These configurations are not set on env variables: "); + this.logger.log(result); + throw new Error("Configuration error, some env variables are not set"); } + return true; } } diff --git a/lib/env_validator.js b/lib/env_validator.js index 954f703..c9a1076 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -20,6 +20,7 @@ const isNil = require("lodash/isNil"); exports.validateEnvironment = (environment, configuration, delimiters = { left: "${", right: "}" }) => { const config = new Config(null); if (isNil(environment)) throw new Error("No environment variable installed in this system"); + if (isNil(configuration)) return []; // no need to check if no configuration const flattenConfiguration = config._flatten(configuration); @@ -29,9 +30,7 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: if (isNil(value)) { throw new Error(`Error on Config, '${key}' is needed, but the value is ${value}`); } - - if (value.includes(delimiters.left) && - value.includes(delimiters.right)) { + if (containDelimiters(value, delimiters)) { const envKey = value.substring(delimiters.left.length, value.length - delimiters.right.length); const envValue = environment[envKey]; @@ -43,3 +42,10 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: return neededValues; }; +const containDelimiters = (string, delimiters) => { + if (isNil(string)) return false; + return typeof string === "string" && + string.includes(delimiters.left) && + string.includes(delimiters.right); +}; +exports.containDelimiters = containDelimiters; diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index 7b454b2..fc796a7 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -65,3 +65,7 @@ describe("Env validator", () => { } }); }); + +describe("", () => { + +}); From bd4f7efa2621314118f231a9fd1897cdfe5ce9c3 Mon Sep 17 00:00:00 2001 From: mamaz Date: Wed, 26 Dec 2018 18:11:00 +0700 Subject: [PATCH 05/16] Add tests for containDelimiters --- lib/env_validator.js | 5 +++-- test/env_validator.spec.js | 31 ++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/lib/env_validator.js b/lib/env_validator.js index c9a1076..e630462 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -18,13 +18,13 @@ const isNil = require("lodash/isNil"); * */ exports.validateEnvironment = (environment, configuration, delimiters = { left: "${", right: "}" }) => { - const config = new Config(null); if (isNil(environment)) throw new Error("No environment variable installed in this system"); if (isNil(configuration)) return []; // no need to check if no configuration + const config = new Config(null); const flattenConfiguration = config._flatten(configuration); - const neededValues = []; + for (const key of Object.keys(flattenConfiguration)) { const value = flattenConfiguration[key]; if (isNil(value)) { @@ -44,6 +44,7 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: const containDelimiters = (string, delimiters) => { if (isNil(string)) return false; + if (isNil(delimiters)) return false; return typeof string === "string" && string.includes(delimiters.left) && string.includes(delimiters.right); diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index fc796a7..7482927 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -66,6 +66,35 @@ describe("Env validator", () => { }); }); -describe("", () => { +describe("containDelimiters", () => { + let delimiters; + before(() => { + delimiters = {left: "{", right: "}"}; + }); + + it("should return false if string contains NO delimiters", () => { + const res = envValidator.containDelimiters("LALAJO", delimiters); + assert.deepStrictEqual(res, false); + }); + + it("should return true if string contains delimiters", () => { + const res = envValidator.containDelimiters("{LALAJO}", delimiters); + assert.deepStrictEqual(res, true); + }); + it("should return false if string is null / undefined", () => { + let result = envValidator.containDelimiters(null, { left: "{", right: "}" }); + assert.deepStrictEqual(result, false); + + result = envValidator.containDelimiters(undefined, delimiters); + assert.deepStrictEqual(result, false); + }); + + it("should return false if delimiters is null / undefined", () => { + let res = envValidator.containDelimiters("{LALAJO}", null); + assert.deepStrictEqual(res, false); + + res = envValidator.containDelimiters("{LALAJO}", undefined); + assert.deepStrictEqual(res, false); + }); }); From f3ef6e15e4bbdb464cee950ca94107882bea6905 Mon Sep 17 00:00:00 2001 From: mamaz Date: Thu, 27 Dec 2018 10:41:16 +0700 Subject: [PATCH 06/16] Update handlers and add more test --- lib/env_validator.js | 7 ++++--- test/env_validator.spec.js | 11 ++++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/env_validator.js b/lib/env_validator.js index e630462..1cf61c4 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -1,6 +1,7 @@ "use strict"; const Config = require("./config"); const isNil = require("lodash/isNil"); +const isEmpty = require("lodash/isEmpty"); /** * Make sure environment variables needed in configuration exists. @@ -18,10 +19,10 @@ const isNil = require("lodash/isNil"); * */ exports.validateEnvironment = (environment, configuration, delimiters = { left: "${", right: "}" }) => { - if (isNil(environment)) throw new Error("No environment variable installed in this system"); - if (isNil(configuration)) return []; // no need to check if no configuration + if (isNil(environment)) throw new Error("No environment variable set in this system"); + if (isNil(configuration) || isEmpty(environment)) throw new Error("No configuration is set"); - const config = new Config(null); + const config = new Config(); const flattenConfiguration = config._flatten(configuration); const neededValues = []; diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index 7482927..c5fdde2 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -61,7 +61,16 @@ describe("Env validator", () => { envValidator.validateEnvironment(null, config, delimiters); assert.fail("Should throw error on Config"); } catch(e) { - assert.equal(e.message, "No environment variable installed in this system"); + assert.equal(e.message, "No environment variable set in this system"); + } + }); + + it("should throw error if no configuration is set", () => { + try { + envValidator.validateEnvironment({}, null, delimiters); + assert.fail("Should throw error on Config"); + } catch(e) { + assert.equal(e.message, "No configuration is set"); } }); }); From 2871df370b6f92471723be01b3af7f95bd038771 Mon Sep 17 00:00:00 2001 From: mamaz Date: Thu, 27 Dec 2018 12:16:19 +0700 Subject: [PATCH 07/16] Add validateConfig test --- test/env_validator.spec.js | 3 --- test/merapi_test.spec.js | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index c5fdde2..9a95ca7 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -50,7 +50,6 @@ describe("Env validator", () => { }; try { envValidator.validateEnvironment(process.env, config, delimiters); - assert.fail("Should throw error on Config"); } catch(e) { assert.equal(e.message, "Error on Config, 'diaenne.type' is needed, but the value is null"); } @@ -59,7 +58,6 @@ describe("Env validator", () => { it("should throw error if no environment variables is not installed in this system", () => { try { envValidator.validateEnvironment(null, config, delimiters); - assert.fail("Should throw error on Config"); } catch(e) { assert.equal(e.message, "No environment variable set in this system"); } @@ -68,7 +66,6 @@ describe("Env validator", () => { it("should throw error if no configuration is set", () => { try { envValidator.validateEnvironment({}, null, delimiters); - assert.fail("Should throw error on Config"); } catch(e) { assert.equal(e.message, "No configuration is set"); } diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index 23a444c..5252834 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -77,6 +77,24 @@ describe("Merapi Test", function() { assert.equal(ENV.NODE_ENV, process.env.NODE_ENV); assert.equal(ENV.PATH, process.env.PATH); }); + + it("can throw error if config value is not set on env variable", asyn(function*() { + container = merapi({ + delimiters: { + left: "${", + right: "}" + }, + config: { + "package.name": "${SOME_NAME}" + } + }); + + try { + yield container.initialize(); + } catch(e) { + assert.equal(e.message, "Configuration error, some env variables are not set"); + } + })); }); describe("Components", function() { From 9a581451a56ec2a4aa0d94e862ea446e7449ead3 Mon Sep 17 00:00:00 2001 From: mamaz Date: Thu, 27 Dec 2018 14:40:26 +0700 Subject: [PATCH 08/16] Cleanup some comment --- lib/env_validator.js | 2 +- test/merapi_test.spec.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/env_validator.js b/lib/env_validator.js index 1cf61c4..33a1509 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -6,7 +6,7 @@ const isEmpty = require("lodash/isEmpty"); /** * Make sure environment variables needed in configuration exists. * - * Return environment variables needed to be defined in process.env. + * Return environment variables needed to be defined. * * @param {object} environment Environment variables to be validated, intended to be filled with process.env object * @param {object} configuration Configuration used for merapi diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index 5252834..f80314b 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -298,7 +298,6 @@ describe("Merapi Test", function() { assert.equal(testVal, 0); yield container.start(); assert.equal(testVal, 1); - })); }); }); \ No newline at end of file From 2a109bfc504e7efc46f16424ae5453b8df8bd294 Mon Sep 17 00:00:00 2001 From: mamaz Date: Fri, 28 Dec 2018 18:44:29 +0700 Subject: [PATCH 09/16] Fix validation params --- index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index b6b05a6..ece524b 100644 --- a/index.js +++ b/index.js @@ -328,7 +328,8 @@ class Container extends Component.mixin(AsyncEmitter) { this.options.extConfig, process.env ); - const result = EnvValidator.validateEnvironment(combinedEnv, this.options.config); + const { config, delimiters } = this.options; + const result = EnvValidator.validateEnvironment(combinedEnv, config, delimiters); if (result.length > 0) { this.logger.log("These configurations are not set on env variables: "); this.logger.log(result); From c3d36898eca5efa087a0cdab0f20f9ac0b49a861 Mon Sep 17 00:00:00 2001 From: mamaz Date: Mon, 31 Dec 2018 13:35:19 +0700 Subject: [PATCH 10/16] Add more test cases for default delimiters --- lib/config.js | 20 ++++++++++---------- lib/env_validator.js | 2 +- test/merapi_test.spec.js | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/lib/config.js b/lib/config.js index 7fe89a3..24300ba 100644 --- a/lib/config.js +++ b/lib/config.js @@ -12,7 +12,7 @@ function trimify(str) { /** * Config creator - * + * * @param {Object} data * Config data */ @@ -109,7 +109,7 @@ class Config { /** * Check if config path exist - * + * * @param {String} path * config path * @returns {Boolean} @@ -118,7 +118,7 @@ class Config { has(path) { return this.get(path, true) !== undefined; } - + /** * Get or use default value * @param {String} path @@ -128,7 +128,7 @@ class Config { default(path, def) { return this.has(path) ? this.get(path) : def; } - + /** * Internal flatten function * @param {Object} data @@ -149,7 +149,7 @@ class Config { } }); } - + return res; } @@ -228,10 +228,10 @@ class Config { } return ret; } - + /** * Create subconfig by path - * + * * @method path * @param {String} path * config path @@ -241,7 +241,7 @@ class Config { path(path, opts) { return this.create(this.get(path), opts); } - + /** * Extend config with data * @param {Object} data @@ -255,7 +255,7 @@ class Config { } return this; } - + /** * Create new config * @param {Object} data @@ -278,7 +278,7 @@ class Config { return config; } - + /** * Create new config * @param {Object} data diff --git a/lib/env_validator.js b/lib/env_validator.js index 33a1509..cae51d6 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -18,7 +18,7 @@ const isEmpty = require("lodash/isEmpty"); * for entry ${$SOME_ENV} * */ -exports.validateEnvironment = (environment, configuration, delimiters = { left: "${", right: "}" }) => { +exports.validateEnvironment = (environment, configuration, delimiters = { left: "{", right: "}" }) => { if (isNil(environment)) throw new Error("No environment variable set in this system"); if (isNil(configuration) || isEmpty(environment)) throw new Error("No configuration is set"); diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index f80314b..a6257ec 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -95,6 +95,44 @@ describe("Merapi Test", function() { assert.equal(e.message, "Configuration error, some env variables are not set"); } })); + + it("can use custom delimiters", asyn(function*() { + container = merapi({ + delimiters: { + left: "[", + right: "]" + }, + config: { + "nameEnv": "[SOME_NAME]" + }, + envConfig: { + test: { + SOME_NAME: "mamazo" + } + } + }); + yield container.initialize(); + + let name = container.config.get("nameEnv"); + assert.equal(name, "mamazo"); + })); + + it("can use default delimiters (left:`{`, right: `}`) if no custom delimiters specified", asyn(function*() { + container = merapi({ + config: { + "nameEnv": "{SOME_NAME}" + }, + envConfig: { + test: { + SOME_NAME: "mamazo" + } + } + }); + yield container.initialize(); + + let name = container.config.get("nameEnv"); + assert.equal(name, "mamazo"); + })); }); describe("Components", function() { From 0529b7c36e3312d9f7218c2e5b20899183801f7f Mon Sep 17 00:00:00 2001 From: mamaz Date: Mon, 31 Dec 2018 14:26:39 +0700 Subject: [PATCH 11/16] Validate also for empty string variable --- lib/env_validator.js | 11 ++++++++--- test/env_validator.spec.js | 24 +++++++++++++++++------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/env_validator.js b/lib/env_validator.js index cae51d6..c7d4c00 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -24,7 +24,10 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: const config = new Config(); const flattenConfiguration = config._flatten(configuration); - const neededValues = []; + const neededValues = { + undefined: [], + empty: [] + }; for (const key of Object.keys(flattenConfiguration)) { const value = flattenConfiguration[key]; @@ -35,8 +38,10 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: const envKey = value.substring(delimiters.left.length, value.length - delimiters.right.length); const envValue = environment[envKey]; - if (isNil(envValue) || envValue === "") { - neededValues.push(envKey); + if (isNil(envValue)) { + neededValues["undefined"].push(envKey); + } else if (envValue === "") { + neededValues["empty"].push(envKey); } } } diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index 9a95ca7..2c72fe8 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -8,6 +8,7 @@ describe("Env validator", () => { let config; process.env.GEIST_URI = "https://example.com"; process.env.GEIST_TOKEN = "asasaklns12io1u31oi2u3"; + const delimiters = { left: "${$", right: "}" @@ -24,13 +25,10 @@ describe("Env validator", () => { }; }); - it("should return empty array if env needed is set already", () => { - const result = envValidator.validateEnvironment(process.env, config, delimiters); - assert.deepStrictEqual(result, []); - }); - - it("should be able to return needed-to-be-set environment variables", () => { + it("should return object of empty and undefined env variables, if not set", () => { + process.env.GEIST_EMPTY = ""; config.geist.lala = "${$LALA}"; + config.geist.empty = "${$GEIST_EMPTY}"; config.diaenne = { type: "proxy", uri: "${$DIAENNE_URI}", @@ -38,8 +36,20 @@ describe("Env validator", () => { }; config.auth = "${$SECRET}"; + const result = { + undefined: ["LALA", "DIAENNE_URI", "VERSION", "SECRET"], + empty: ["GEIST_EMPTY"] + }; + const actualResult = envValidator.validateEnvironment(process.env, config, delimiters); + assert.deepEqual(result, actualResult); + }); + + it("should return empty list of undefined and empty if env needed is set already", () => { const result = envValidator.validateEnvironment(process.env, config, delimiters); - assert.deepStrictEqual(result, ["LALA", "DIAENNE_URI", "VERSION", "SECRET"]); + assert.deepStrictEqual(result, { + undefined: [], + empty: [] + }); }); it("should throw error if one of the variable contains null", () => { From 31445f60c513bbe2f2a47ecca5e5660de36c7c31 Mon Sep 17 00:00:00 2001 From: mamaz Date: Mon, 31 Dec 2018 14:59:31 +0700 Subject: [PATCH 12/16] Add unit test for validateConfig --- index.js | 10 ++++++--- test/merapi_test.spec.js | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index ece524b..12a21c0 100644 --- a/index.js +++ b/index.js @@ -330,9 +330,13 @@ class Container extends Component.mixin(AsyncEmitter) { ); const { config, delimiters } = this.options; const result = EnvValidator.validateEnvironment(combinedEnv, config, delimiters); - if (result.length > 0) { - this.logger.log("These configurations are not set on env variables: "); - this.logger.log(result); + + if (result.empty.length > 0) { + this.logger.warn("These configurations are empty string: ", result.empty); + } + + if (result.undefined.length > 0) { + this.logger.error("These configurations are not set on env variables: ", result.undefined); throw new Error("Configuration error, some env variables are not set"); } return true; diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index a6257ec..3195e6e 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -96,6 +96,54 @@ describe("Merapi Test", function() { } })); + it("should produce warning if some configurations are empty string", asyn(function*() { + process.env.SOME_NAME=""; + container = merapi({ + delimiters: { + left: "${", + right: "}" + }, + config: { + "package.name": "${SOME_NAME}" + } + }); + let a = 0; + container.logger = { + warn: () => { + a = 1; // warn is called + } + }; + + yield container.initialize(); + assert.equal(a, 1); + })); + + it("should produce warning and throw error if some are empty string and some are undefined", asyn(function*() { + process.env.SOME_NAME=""; + container = merapi({ + delimiters: { + left: "${", + right: "}" + }, + config: { + "package.name": "${SOME_NAME}" + } + }); + let a = 0; + container.logger = { + warn: () => { + a = 1; // warn is called + } + }; + + try { + yield container.initialize(); + } catch(e) { + assert.equal(a, 1); + assert.equal(e.message, "Configuration error, some env variables are not set"); + } + })); + it("can use custom delimiters", asyn(function*() { container = merapi({ delimiters: { From 287e5121ed81289841c1747ed9f3defdd8b74570 Mon Sep 17 00:00:00 2001 From: mamaz Date: Mon, 31 Dec 2018 19:34:45 +0700 Subject: [PATCH 13/16] Add $ for system env --- index.js | 9 ++++++++- lib/env_validator.js | 7 ++++--- test/env_validator.spec.js | 18 ++++++++++-------- test/merapi_test.spec.js | 17 +++++++++++++---- 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/index.js b/index.js index 12a21c0..34abc02 100644 --- a/index.js +++ b/index.js @@ -322,11 +322,18 @@ class Container extends Component.mixin(AsyncEmitter) { } validateConfig() { + const systemEnv = () => { + const result = {}; + const env = process.env; + for(const key of Object.keys(env)) + result["$"+key] = env[key]; // system env, append $ to key + return result; + }; const combinedEnv = Object.assign( {}, this.options.envConfig && this.options.envConfig[this.config.env], this.options.extConfig, - process.env + systemEnv() ); const { config, delimiters } = this.options; const result = EnvValidator.validateEnvironment(combinedEnv, config, delimiters); diff --git a/lib/env_validator.js b/lib/env_validator.js index c7d4c00..270830f 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -12,7 +12,7 @@ const isEmpty = require("lodash/isEmpty"); * @param {object} configuration Configuration used for merapi * @param {object} delimiters Delimiters object, used to parse variables. Examples: * formatted in { - * left: "${$" + * left: "${" * right: "}" * } * for entry ${$SOME_ENV} @@ -37,11 +37,12 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: if (containDelimiters(value, delimiters)) { const envKey = value.substring(delimiters.left.length, value.length - delimiters.right.length); const envValue = environment[envKey]; + const sanitisedEnvKey = envKey.replace(/\$/,""); // remove $ if (isNil(envValue)) { - neededValues["undefined"].push(envKey); + neededValues["undefined"].push(sanitisedEnvKey); } else if (envValue === "") { - neededValues["empty"].push(envKey); + neededValues["empty"].push(sanitisedEnvKey); } } } diff --git a/test/env_validator.spec.js b/test/env_validator.spec.js index 2c72fe8..5688b39 100644 --- a/test/env_validator.spec.js +++ b/test/env_validator.spec.js @@ -6,11 +6,13 @@ const envValidator = require("../lib/env_validator"); describe("Env validator", () => { let config; - process.env.GEIST_URI = "https://example.com"; - process.env.GEIST_TOKEN = "asasaklns12io1u31oi2u3"; + let env = { + "$GEIST_URI": "https://example.com", + "$GEIST_TOKEN": "asasaklns12io1u31oi2u3" + }; const delimiters = { - left: "${$", + left: "${", right: "}" }; @@ -26,7 +28,7 @@ describe("Env validator", () => { }); it("should return object of empty and undefined env variables, if not set", () => { - process.env.GEIST_EMPTY = ""; + env["$GEIST_EMPTY"] = ""; config.geist.lala = "${$LALA}"; config.geist.empty = "${$GEIST_EMPTY}"; config.diaenne = { @@ -40,12 +42,12 @@ describe("Env validator", () => { undefined: ["LALA", "DIAENNE_URI", "VERSION", "SECRET"], empty: ["GEIST_EMPTY"] }; - const actualResult = envValidator.validateEnvironment(process.env, config, delimiters); - assert.deepEqual(result, actualResult); + const actualResult = envValidator.validateEnvironment(env, config, delimiters); + assert.deepEqual(actualResult, result); }); it("should return empty list of undefined and empty if env needed is set already", () => { - const result = envValidator.validateEnvironment(process.env, config, delimiters); + const result = envValidator.validateEnvironment(env, config, delimiters); assert.deepStrictEqual(result, { undefined: [], empty: [] @@ -59,7 +61,7 @@ describe("Env validator", () => { version: "${$VERSION}" }; try { - envValidator.validateEnvironment(process.env, config, delimiters); + envValidator.validateEnvironment(env, config, delimiters); } catch(e) { assert.equal(e.message, "Error on Config, 'diaenne.type' is needed, but the value is null"); } diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index 3195e6e..fe088e0 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -17,6 +17,7 @@ describe("Merapi Test", function() { let container = null; beforeEach(asyn(function*() { + process.env.TOKEN="123"; container = merapi({ delimiters: { left: "${", @@ -27,7 +28,8 @@ describe("Merapi Test", function() { "myConfig": "${resolved.a}", "myEnvConfig": "${resolved.b}", "myStrEnvConfig": "${resolved.c}", - "myCrlfStrEnvConfig": "${resolved.d}" + "myCrlfStrEnvConfig": "${resolved.d}", + "myToken": "${$TOKEN}" // for system env variables, $ is appended }, envConfig: { @@ -47,7 +49,7 @@ describe("Merapi Test", function() { yield container.initialize(); })); - it("can resolve config", function() { + it("can resolve config from envConfig", function() { assert.notEqual(container, null); let myConfig = container.config.get("myConfig"); @@ -56,6 +58,13 @@ describe("Merapi Test", function() { assert.equal(pkg.name, "merapi-test"); }); + it("can resolve config from system env variables", () => { + assert.notEqual(container, null); + + const myToken = container.config.get("myToken"); + assert.equal(myToken, 123); + }); + it("can resolve environment config", function() { let myEnvConfig = container.config.get("myEnvConfig"); let myStrEnvConfig = container.config.get("myStrEnvConfig"); @@ -104,7 +113,7 @@ describe("Merapi Test", function() { right: "}" }, config: { - "package.name": "${SOME_NAME}" + "package.name": "${$SOME_NAME}" } }); let a = 0; @@ -126,7 +135,7 @@ describe("Merapi Test", function() { right: "}" }, config: { - "package.name": "${SOME_NAME}" + "package.name": "${$SOME_NAME}" } }); let a = 0; From 096967fa53bff6f998e3790f866c8ac9b19a92b5 Mon Sep 17 00:00:00 2001 From: mamaz Date: Wed, 2 Jan 2019 17:58:46 +0700 Subject: [PATCH 14/16] Add validation for nested values --- lib/config.js | 3 +-- lib/env_validator.js | 20 +++++++++++++++++--- test/merapi_test.spec.js | 8 +++++--- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/lib/config.js b/lib/config.js index 24300ba..8c0aa70 100644 --- a/lib/config.js +++ b/lib/config.js @@ -60,7 +60,6 @@ class Config { } data = val; } - return data; } /** @@ -192,7 +191,6 @@ class Config { } return tpl(params); } - return tpl(params); } @@ -226,6 +224,7 @@ class Config { for (let n in flat) { ret[n] = this.set(n, this.resolve(n, false)); } + return ret; } diff --git a/lib/env_validator.js b/lib/env_validator.js index 270830f..c0d95c9 100644 --- a/lib/env_validator.js +++ b/lib/env_validator.js @@ -39,10 +39,10 @@ exports.validateEnvironment = (environment, configuration, delimiters = { left: const envValue = environment[envKey]; const sanitisedEnvKey = envKey.replace(/\$/,""); // remove $ - if (isNil(envValue)) { - neededValues["undefined"].push(sanitisedEnvKey); - } else if (envValue === "") { + if (envValue === "") { neededValues["empty"].push(sanitisedEnvKey); + } else if (isNil(envValue) && !isNestedValue(envKey, configuration)) { + neededValues["undefined"].push(sanitisedEnvKey); } } } @@ -57,3 +57,17 @@ const containDelimiters = (string, delimiters) => { string.includes(delimiters.right); }; exports.containDelimiters = containDelimiters; + +const isNestedValue = (value, config) => { + let data = config; + const parts = value.split(".").map(val => /^\d+$/.test(val) ? parseInt(val) : val); + if (parts.length === 1) return false; + for(let i = 0; i < parts.length; i++) { + let value = data[parts[i]]; + if (data === undefined) { + return false; + } + data = value; + } + return true; +}; diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index fe088e0..ba8b1dd 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -29,13 +29,15 @@ describe("Merapi Test", function() { "myEnvConfig": "${resolved.b}", "myStrEnvConfig": "${resolved.c}", "myCrlfStrEnvConfig": "${resolved.d}", - "myToken": "${$TOKEN}" // for system env variables, $ is appended + "myToken": "${$TOKEN}", // for system env variables, $ is appended + "resolved": { + "a": 1 + } }, envConfig: { test: { - "resolved.a": 1, - "resolved.b": 2, + "resolved.b": 2, "resolved.c": "test", "resolved.d": "test\r", } From 20818cc3f24acf4a1ed8ecbd55d8fc46a1ffac81 Mon Sep 17 00:00:00 2001 From: mamaz Date: Thu, 3 Jan 2019 13:24:26 +0700 Subject: [PATCH 15/16] Fix space indenting --- test/merapi_test.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/merapi_test.spec.js b/test/merapi_test.spec.js index ba8b1dd..ade2ef8 100644 --- a/test/merapi_test.spec.js +++ b/test/merapi_test.spec.js @@ -37,7 +37,7 @@ describe("Merapi Test", function() { envConfig: { test: { - "resolved.b": 2, + "resolved.b": 2, "resolved.c": "test", "resolved.d": "test\r", } From 3014ec3748561f6c1db9af8dbf57f4cfb32f4404 Mon Sep 17 00:00:00 2001 From: mamaz Date: Mon, 7 Jan 2019 11:28:09 +0700 Subject: [PATCH 16/16] Add more subtle warning --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 34abc02..692790f 100644 --- a/index.js +++ b/index.js @@ -339,7 +339,7 @@ class Container extends Component.mixin(AsyncEmitter) { const result = EnvValidator.validateEnvironment(combinedEnv, config, delimiters); if (result.empty.length > 0) { - this.logger.warn("These configurations are empty string: ", result.empty); + this.logger.warn("WARNING! These configurations are empty string: ", result.empty); } if (result.undefined.length > 0) {