From 6e6fea3fa2df59f38bcad0ed539999e833faeed0 Mon Sep 17 00:00:00 2001 From: Benjamin Forster Date: Fri, 1 Dec 2017 10:33:30 +1100 Subject: [PATCH] update ssm cache to generic variable cache. --- lib/classes/Variables.js | 51 ++++++++++++++++-------------- lib/classes/Variables.test.js | 58 +++++++++++++++++------------------ 2 files changed, 57 insertions(+), 52 deletions(-) diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index ee8a308c1..f4932f49d 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -13,6 +13,7 @@ class Variables { constructor(serverless) { this.serverless = serverless; this.service = this.serverless.service; + this.cache = {}; this.overwriteSyntax = RegExp(/,/g); this.fileRefSyntax = RegExp(/^file\((~?[a-zA-Z0-9._\-/]+?)\)/g); @@ -23,8 +24,6 @@ class Variables { this.s3RefSyntax = RegExp(/^s3:(.+?)\/(.+)$/); this.stringRefSynax = RegExp(/('.*')|(".*")/g); this.ssmRefSyntax = RegExp(/^ssm:([a-zA-Z0-9_.\-/]+)[~]?(true|false)?/); - - this.ssmCache = {}; } loadVariableSyntax() { @@ -169,23 +168,33 @@ class Variables { } getValueFromSource(variableString) { - if (variableString.match(this.envRefSyntax)) { - return this.getValueFromEnv(variableString); - } else if (variableString.match(this.optRefSyntax)) { - return this.getValueFromOptions(variableString); - } else if (variableString.match(this.selfRefSyntax)) { - return this.getValueFromSelf(variableString); - } else if (variableString.match(this.fileRefSyntax)) { - return this.getValueFromFile(variableString); - } else if (variableString.match(this.cfRefSyntax)) { - return this.getValueFromCf(variableString); - } else if (variableString.match(this.s3RefSyntax)) { - return this.getValueFromS3(variableString); - } else if (variableString.match(this.stringRefSynax)) { - return this.getValueFromString(variableString); - } else if (variableString.match(this.ssmRefSyntax)) { - return this.getValueFromSsm(variableString); + const cached = this.cache[variableString]; + if (cached) { + return cached; } + let valueFromSource; + if (variableString.match(this.envRefSyntax)) { + valueFromSource = this.getValueFromEnv(variableString); + } else if (variableString.match(this.optRefSyntax)) { + valueFromSource = this.getValueFromOptions(variableString); + } else if (variableString.match(this.selfRefSyntax)) { + valueFromSource = this.getValueFromSelf(variableString); + } else if (variableString.match(this.fileRefSyntax)) { + valueFromSource = this.getValueFromFile(variableString); + } else if (variableString.match(this.cfRefSyntax)) { + valueFromSource = this.getValueFromCf(variableString); + } else if (variableString.match(this.s3RefSyntax)) { + valueFromSource = this.getValueFromS3(variableString); + } else if (variableString.match(this.stringRefSynax)) { + valueFromSource = this.getValueFromString(variableString); + } else if (variableString.match(this.ssmRefSyntax)) { + valueFromSource = this.getValueFromSsm(variableString); + } + if (valueFromSource) { + this.cache[variableString] = valueFromSource; + return valueFromSource; + } + const errorMessage = [ `Invalid variable reference syntax for variable ${variableString}.`, ' You can only reference env vars, options, & files.', @@ -367,13 +376,11 @@ class Variables { } getValueFromSsm(variableString) { - const cached = this.ssmCache[variableString]; - if (cached) return cached; const groups = variableString.match(this.ssmRefSyntax); const param = groups[1]; const decrypt = (groups[2] === 'true'); - const promise = this.serverless.getProvider('aws') + return this.serverless.getProvider('aws') .request('SSM', 'getParameter', { @@ -392,8 +399,6 @@ class Variables { return BbPromise.resolve(undefined); } ); - this.ssmCache[variableString] = promise; - return promise; } getDeepValue(deepProperties, valueToPopulate) { diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index d7be64e55..636633935 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -512,6 +512,35 @@ describe('Variables', () => { .finally(() => serverless.variables.getValueFromSsm.restore()); }); + describe('caching', () => { + const sources = [ + { function: 'getValueFromEnv', variableString: 'env:NODE_ENV' }, + { function: 'getValueFromOptions', variableString: 'opt:stage' }, + { function: 'getValueFromSelf', variableString: 'self:provider' }, + { function: 'getValueFromFile', variableString: 'file(./config.yml)' }, + { function: 'getValueFromCf', variableString: 'cf:test-stack.testOutput' }, + { function: 'getValueFromS3', variableString: 's3:test-bucket/path/to/ke' }, + { function: 'getValueFromSsm', variableString: 'ssm:/test/path/to/param' }, + ]; + sources.forEach((source) => { + it(`should only call ${source.function} once, returning the cached value otherwise`, () => { + const serverless = new Serverless(); + const getValueFromSsmStub = sinon + .stub(serverless.variables, source.function).resolves('variableValue'); + const firstCall = serverless.variables.getValueFromSource(source.variableString); + const secondCall = BbPromise.delay(100) + .then(() => serverless.variables.getValueFromSource(source.variableString)); + return BbPromise.all([firstCall, secondCall]) + .then(valueToPopulate => { + expect(valueToPopulate).to.deep.equal(['variableValue', 'variableValue']); + expect(getValueFromSsmStub).to.have.been.calledOnce; + expect(getValueFromSsmStub).to.have.been.calledWith(source.variableString); + }) + .finally(() => serverless.variables[source.function].restore()); + }); + }); + }); + it('should throw error if referencing an invalid source', () => { const serverless = new Serverless(); expect(() => serverless.variables.getValueFromSource('weird:source')) @@ -1049,35 +1078,6 @@ describe('Variables', () => { }); }); - it('cache variables from Ssm', () => { - const param = '/path/to/Param-01_valid.chars'; - const value = 'MockValue'; - const awsResponseMock = { - Parameter: { - Value: value, - }, - }; - const ssmStub = sinon.stub(awsProvider, 'request').resolves(awsResponseMock); - const firstCall = serverless.variables.getValueFromSsm(`ssm:${param}`); - const secondCall = BbPromise.delay(100) - .then(() => serverless.variables.getValueFromSsm(`ssm:${param}`)); - return BbPromise.all([firstCall, secondCall]) - .then((resolved) => { - expect(resolved).to.be.deep.equal([value, value]); - expect(ssmStub).to.have.been.calledOnce; - expect(ssmStub).to.have.been.calledWithExactly( - 'SSM', - 'getParameter', - { - Name: param, - WithDecryption: false, - }, - serverless.variables.options.stage, - serverless.variables.options.region - ); - }); - }); - it('should get encrypted variable from Ssm using extended syntax', () => { const param = '/path/to/Param-01_valid.chars'; const value = 'MockValue';