From 7219d188530d60863fb1b76fa1ae7c851b061d87 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Thu, 3 Sep 2020 07:55:32 -0400 Subject: [PATCH 1/4] Add failing test for port coercion --- packages/config-loader/src/loader.test.ts | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index 66894c7dec..5246722538 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/packages/config-loader/src/loader.test.ts @@ -30,6 +30,14 @@ jest.mock('fs-extra', () => { sessionKey: development-key `, '/root/secrets/session-key.txt': 'abc123', + '/secret-port/app-config.yaml': ` + backend: + listen: + port: + $secret: + file: secrets/port.txt + `, + '/secret-port/secrets/port.txt': '12345', }; return { @@ -137,4 +145,25 @@ describe('loadConfig', () => { }, ]); }); + + it('coerces port to a number', async () => { + await expect( + loadConfig({ + rootPaths: ['/secret-port'], + env: 'production', + shouldReadSecrets: true, + }), + ).resolves.toEqual([ + { + context: 'app-config.yaml', + data: { + backend: { + listen: { + port: 12345, + }, + }, + }, + }, + ]); + }); }); From f955f92ee6e70bff9dd70a7977d8dd094434b8d0 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Thu, 3 Sep 2020 08:06:08 -0400 Subject: [PATCH 2/4] Add failed test for coercion of numberic strings --- packages/config/src/reader.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 41cdc92938..3fa8bb5860 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -35,6 +35,7 @@ const DATA = { strings: ['string1', 'string2'], }, nestlings: [{ boolean: true }, { string: 'string' }, { number: 42 }] as {}[], + port: 'number', }; function expectValidValues(config: ConfigReader) { @@ -580,4 +581,17 @@ describe('ConfigReader.get()', () => { }, }); }); + + it('coerces number strings to numbers', () => { + const config = ConfigReader.fromConfigs([ + { + data: { + port: '123', + }, + context: '1', + }, + ]); + + expect(config.getNumber('port')).toEqual(123); + }); }); From 3877a50ce9772b495a504ed58faff4fca088d242 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Thu, 3 Sep 2020 08:30:40 -0400 Subject: [PATCH 3/4] Remove unnecessary line --- packages/config/src/reader.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 3fa8bb5860..48253dcebe 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -35,7 +35,6 @@ const DATA = { strings: ['string1', 'string2'], }, nestlings: [{ boolean: true }, { string: 'string' }, { number: 42 }] as {}[], - port: 'number', }; function expectValidValues(config: ConfigReader) { From 265be1d6a1f5f34bc09d86f83a9c9f126371c0b4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 3 Sep 2020 16:35:16 +0200 Subject: [PATCH 4/4] config: try to coerce string values into numbers --- packages/config-loader/src/loader.test.ts | 21 --------------------- packages/config/src/reader.test.ts | 2 +- packages/config/src/reader.ts | 19 +++++++++++++++++-- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/packages/config-loader/src/loader.test.ts b/packages/config-loader/src/loader.test.ts index 5246722538..4a7c9f238c 100644 --- a/packages/config-loader/src/loader.test.ts +++ b/packages/config-loader/src/loader.test.ts @@ -145,25 +145,4 @@ describe('loadConfig', () => { }, ]); }); - - it('coerces port to a number', async () => { - await expect( - loadConfig({ - rootPaths: ['/secret-port'], - env: 'production', - shouldReadSecrets: true, - }), - ).resolves.toEqual([ - { - context: 'app-config.yaml', - data: { - backend: { - listen: { - port: 12345, - }, - }, - }, - }, - ]); - }); }); diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 48253dcebe..3fe74aa637 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -88,7 +88,7 @@ function expectInvalidValues(config: ConfigReader) { "Invalid type in config for key 'string' in 'ctx', got string, wanted boolean", ); expect(() => config.getNumber('string')).toThrow( - "Invalid type in config for key 'string' in 'ctx', got string, wanted number", + "Unable to convert config value for key 'string' in 'ctx' to a number", ); expect(() => config.getString('one')).toThrow( "Invalid type in config for key 'one' in 'ctx', got number, wanted string", diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 65143bbd95..eb8c91e366 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -49,6 +49,9 @@ const errors = { missing(key: string) { return `Missing required config value at '${key}'`; }, + convert(key: string, context: string, expected: string) { + return `Unable to convert config value for key '${key}' in '${context}' to a ${expected}`; + }, }; export class ConfigReader implements Config { @@ -183,10 +186,22 @@ export class ConfigReader implements Config { } getOptionalNumber(key: string): number | undefined { - return this.readConfigValue( + const value = this.readConfigValue( key, - value => typeof value === 'number' || { expected: 'number' }, + val => + typeof val === 'number' || + typeof val === 'string' || { expected: 'number' }, ); + if (typeof value === 'number' || value === undefined) { + return value; + } + const number = Number(value); + if (!Number.isFinite(number)) { + throw new Error( + errors.convert(this.fullKey(key), this.context, 'number'), + ); + } + return number; } getBoolean(key: string): boolean {