From c63fa187a360b166bcab2aae49316a775aa903f2 Mon Sep 17 00:00:00 2001 From: Erick Daniszewski Date: Mon, 22 Jun 2020 22:34:11 -0400 Subject: [PATCH 1/4] feat: cast environment variable values to string --- package/lib/compileFunctions.js | 11 +++++++---- package/lib/compileFunctions.test.js | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/package/lib/compileFunctions.js b/package/lib/compileFunctions.js index b318d47..8415180 100644 --- a/package/lib/compileFunctions.js +++ b/package/lib/compileFunctions.js @@ -47,10 +47,13 @@ module.exports = { 'nodejs8'; funcTemplate.properties.timeout = _.get(funcObject, 'timeout') || _.get(this, 'serverless.service.provider.timeout') || '60s'; - funcTemplate.properties.environmentVariables = _.merge( - {}, - _.get(this, 'serverless.service.provider.environment'), - funcObject.environment // eslint-disable-line comma-dangle + funcTemplate.properties.environmentVariables = _.mapValues( + _.merge( + {}, + _.get(this, 'serverless.service.provider.environment'), + funcObject.environment // eslint-disable-line comma-dangle + ), + (value) => value.toString() ); funcTemplate.accessControl.gcpIamPolicy.bindings = _.unionBy( _.get(funcObject, 'iam.bindings'), diff --git a/package/lib/compileFunctions.test.js b/package/lib/compileFunctions.test.js index a4c764d..befd929 100644 --- a/package/lib/compileFunctions.test.js +++ b/package/lib/compileFunctions.test.js @@ -376,6 +376,9 @@ describe('CompileFunctions', () => { handler: 'func1', environment: { TEST_VAR: 'test', + INT_VAR: 1, + FLOAT_VAR: 3.141, + BOOL_VAR: true, }, events: [{ http: 'foo' }], }, @@ -393,6 +396,9 @@ describe('CompileFunctions', () => { availableMemoryMb: 256, environmentVariables: { TEST_VAR: 'test', + INT_VAR: '1', + FLOAT_VAR: '3.141', + BOOL_VAR: 'true', }, timeout: '60s', sourceArchiveUrl: 'gs://sls-my-service-dev-12345678/some-path/artifact.zip', @@ -421,6 +427,9 @@ describe('CompileFunctions', () => { }; googlePackage.serverless.service.provider.environment = { TEST_VAR: 'test', + INT_VAR: 1, + FLOAT_VAR: 3.141, + BOOL_VAR: true, }; const compiledResources = [ @@ -435,6 +444,9 @@ describe('CompileFunctions', () => { availableMemoryMb: 256, environmentVariables: { TEST_VAR: 'test', + INT_VAR: '1', + FLOAT_VAR: '3.141', + BOOL_VAR: 'true', }, timeout: '60s', sourceArchiveUrl: 'gs://sls-my-service-dev-12345678/some-path/artifact.zip', @@ -461,6 +473,7 @@ describe('CompileFunctions', () => { environment: { TEST_VAR: 'test_var', TEST_VALUE: 'foobar', + TEST_BOOL: true, }, events: [{ http: 'foo' }], }, @@ -468,6 +481,7 @@ describe('CompileFunctions', () => { googlePackage.serverless.service.provider.environment = { TEST_VAR: 'test', TEST_FOO: 'foo', + TEST_BOOL: false, }; const compiledResources = [ @@ -484,6 +498,7 @@ describe('CompileFunctions', () => { TEST_VAR: 'test_var', TEST_VALUE: 'foobar', TEST_FOO: 'foo', + TEST_BOOL: 'true', }, timeout: '60s', sourceArchiveUrl: 'gs://sls-my-service-dev-12345678/some-path/artifact.zip', @@ -503,6 +518,7 @@ describe('CompileFunctions', () => { expect(googlePackage.serverless.service.provider.environment).toEqual({ TEST_VAR: 'test', TEST_FOO: 'foo', + TEST_BOOL: false, }); }); }); From f65c7cf8016a7e8d0ba4bbb807345ff52393d929 Mon Sep 17 00:00:00 2001 From: Erick Daniszewski Date: Wed, 24 Jun 2020 14:47:40 -0400 Subject: [PATCH 2/4] update: coerce numbers to string, error on other non-string types for env values --- package/lib/compileFunctions.js | 25 ++++++++--- package/lib/compileFunctions.test.js | 66 ++++++++++++++++++++++++---- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/package/lib/compileFunctions.js b/package/lib/compileFunctions.js index 8415180..dd6dd6e 100644 --- a/package/lib/compileFunctions.js +++ b/package/lib/compileFunctions.js @@ -47,13 +47,10 @@ module.exports = { 'nodejs8'; funcTemplate.properties.timeout = _.get(funcObject, 'timeout') || _.get(this, 'serverless.service.provider.timeout') || '60s'; - funcTemplate.properties.environmentVariables = _.mapValues( - _.merge( - {}, - _.get(this, 'serverless.service.provider.environment'), - funcObject.environment // eslint-disable-line comma-dangle - ), - (value) => value.toString() + funcTemplate.properties.environmentVariables = _.transform( + _.merge({}, _.get(this, 'serverless.service.provider.environment'), funcObject.environment), + (result, value, key) => coerceEnvOrError(result, key, value), + {} ); funcTemplate.accessControl.gcpIamPolicy.bindings = _.unionBy( _.get(funcObject, 'iam.bindings'), @@ -201,6 +198,20 @@ const validateIamProperty = (funcObject, functionName) => { } }; +const coerceEnvOrError = (result, key, value) => { + if (typeof value === 'string') { + result[key] = value; + } else if (typeof value === 'number') { + result[key] = value.toString(); + } else { + const errorMessage = [ + `The value for environment variable ${key} is an unsupported type: ${typeof value}.`, + ' Values must either be strings or numbers (which are coerced into strings at package time).', + ].join(''); + throw new Error(errorMessage); + } +}; + const getFunctionTemplate = (funcObject, projectName, region, sourceArchiveUrl) => { //eslint-disable-line return { diff --git a/package/lib/compileFunctions.test.js b/package/lib/compileFunctions.test.js index befd929..78cb02b 100644 --- a/package/lib/compileFunctions.test.js +++ b/package/lib/compileFunctions.test.js @@ -370,6 +370,64 @@ describe('CompileFunctions', () => { }); }); + it('should fail setting environment variable due to unsupported type (bool)', () => { + googlePackage.serverless.service.functions = { + func1: { + handler: 'func1', + environment: { + TEST_VAR: true, + }, + events: [{ http: 'foo' }], + }, + }; + + expect(() => googlePackage.compileFunctions()).toThrow(Error); + }); + + it('should fail setting environment variable due to unsupported type (null)', () => { + googlePackage.serverless.service.functions = { + func1: { + handler: 'func1', + environment: { + TEST_VAR: null, + }, + events: [{ http: 'foo' }], + }, + }; + + expect(() => googlePackage.compileFunctions()).toThrow(Error); + }); + + it('should fail setting environment variable due to unsupported type (object)', () => { + googlePackage.serverless.service.functions = { + func1: { + handler: 'func1', + environment: { + dev: { + TEST_VAR: 'test', + }, + }, + events: [{ http: 'foo' }], + }, + }; + + expect(() => googlePackage.compileFunctions()).toThrow(Error); + }); + + it('should fail setting environment variable from provider due to unsupported type (bool)', () => { + googlePackage.serverless.service.functions = { + func1: { + handler: 'func1', + events: [{ http: 'foo' }], + }, + }; + googlePackage.serverless.service.provider.environment = { + TEST_VAR: true, + }; + + expect(() => googlePackage.compileFunctions()).toThrow(Error); + }); + it('should set the environment variables based on the function configuration', () => { googlePackage.serverless.service.functions = { func1: { @@ -378,7 +436,6 @@ describe('CompileFunctions', () => { TEST_VAR: 'test', INT_VAR: 1, FLOAT_VAR: 3.141, - BOOL_VAR: true, }, events: [{ http: 'foo' }], }, @@ -398,7 +455,6 @@ describe('CompileFunctions', () => { TEST_VAR: 'test', INT_VAR: '1', FLOAT_VAR: '3.141', - BOOL_VAR: 'true', }, timeout: '60s', sourceArchiveUrl: 'gs://sls-my-service-dev-12345678/some-path/artifact.zip', @@ -429,7 +485,6 @@ describe('CompileFunctions', () => { TEST_VAR: 'test', INT_VAR: 1, FLOAT_VAR: 3.141, - BOOL_VAR: true, }; const compiledResources = [ @@ -446,7 +501,6 @@ describe('CompileFunctions', () => { TEST_VAR: 'test', INT_VAR: '1', FLOAT_VAR: '3.141', - BOOL_VAR: 'true', }, timeout: '60s', sourceArchiveUrl: 'gs://sls-my-service-dev-12345678/some-path/artifact.zip', @@ -473,7 +527,6 @@ describe('CompileFunctions', () => { environment: { TEST_VAR: 'test_var', TEST_VALUE: 'foobar', - TEST_BOOL: true, }, events: [{ http: 'foo' }], }, @@ -481,7 +534,6 @@ describe('CompileFunctions', () => { googlePackage.serverless.service.provider.environment = { TEST_VAR: 'test', TEST_FOO: 'foo', - TEST_BOOL: false, }; const compiledResources = [ @@ -498,7 +550,6 @@ describe('CompileFunctions', () => { TEST_VAR: 'test_var', TEST_VALUE: 'foobar', TEST_FOO: 'foo', - TEST_BOOL: 'true', }, timeout: '60s', sourceArchiveUrl: 'gs://sls-my-service-dev-12345678/some-path/artifact.zip', @@ -518,7 +569,6 @@ describe('CompileFunctions', () => { expect(googlePackage.serverless.service.provider.environment).toEqual({ TEST_VAR: 'test', TEST_FOO: 'foo', - TEST_BOOL: false, }); }); }); From 46fe9b991cb674561b02b710d7f714af7c070e56 Mon Sep 17 00:00:00 2001 From: Erick Daniszewski Date: Thu, 25 Jun 2020 09:36:33 -0400 Subject: [PATCH 3/4] update: use mapValues instead of transform --- package/lib/compileFunctions.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/package/lib/compileFunctions.js b/package/lib/compileFunctions.js index dd6dd6e..650018c 100644 --- a/package/lib/compileFunctions.js +++ b/package/lib/compileFunctions.js @@ -47,10 +47,9 @@ module.exports = { 'nodejs8'; funcTemplate.properties.timeout = _.get(funcObject, 'timeout') || _.get(this, 'serverless.service.provider.timeout') || '60s'; - funcTemplate.properties.environmentVariables = _.transform( + funcTemplate.properties.environmentVariables = _.mapValues( _.merge({}, _.get(this, 'serverless.service.provider.environment'), funcObject.environment), - (result, value, key) => coerceEnvOrError(result, key, value), - {} + (value, key, result) => coerceEnvOrError(result, key, value) ); funcTemplate.accessControl.gcpIamPolicy.bindings = _.unionBy( _.get(funcObject, 'iam.bindings'), From 3fd36865e99e326e4bbaa5806d7b5c0bd9367c1d Mon Sep 17 00:00:00 2001 From: Erick Daniszewski Date: Thu, 25 Jun 2020 10:36:57 -0400 Subject: [PATCH 4/4] fix: fix env coerce function to work with mapValues --- package/lib/compileFunctions.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/package/lib/compileFunctions.js b/package/lib/compileFunctions.js index 650018c..55e43d9 100644 --- a/package/lib/compileFunctions.js +++ b/package/lib/compileFunctions.js @@ -199,16 +199,15 @@ const validateIamProperty = (funcObject, functionName) => { const coerceEnvOrError = (result, key, value) => { if (typeof value === 'string') { - result[key] = value; + return value; } else if (typeof value === 'number') { - result[key] = value.toString(); - } else { - const errorMessage = [ - `The value for environment variable ${key} is an unsupported type: ${typeof value}.`, - ' Values must either be strings or numbers (which are coerced into strings at package time).', - ].join(''); - throw new Error(errorMessage); + return value.toString(); } + const errorMessage = [ + `The value for environment variable ${key} is an unsupported type: ${typeof value}.`, + ' Values must either be strings or numbers (which are coerced into strings at package time).', + ].join(''); + throw new Error(errorMessage); }; const getFunctionTemplate = (funcObject, projectName, region, sourceArchiveUrl) => {