diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js index 4926f31de..764b34924 100644 --- a/lib/classes/Variables.js +++ b/lib/classes/Variables.js @@ -72,25 +72,24 @@ class Variables { // ############# // ## SERVICE ## // ############# - prepopulateObject(objectToPopulate) { + disableDepedentServices(func) { const dependentServices = [ { name: 'CloudFormation', method: 'getValueFromCf', original: this.getValueFromCf }, { name: 'S3', method: 'getValueFromS3', original: this.getValueFromS3 }, { name: 'SSM', method: 'getValueFromSsm', original: this.getValueFromSsm }, ]; - const dependencyMessage = (configName, configValue, serviceName) => - `Variable Failure: value ${configName} set to '${configValue}' references ${ - serviceName} which requires a ${configName} value for use.`; + const dependencyMessage = (configValue, serviceName) => + `Variable dependency failure: variable '${configValue}' references service ${ + serviceName} but using that service requires a concrete value to be called.`; // replace and then restore the methods for obtaining values from dependent services. the // replacement naturally rejects dependencies on these services that occur during prepopulation. // prepopulation is, of course, the process of obtaining the required configuration for using // these services. dependentServices.forEach((dependentService) => { // knock out this[dependentService.method] = (variableString) => BbPromise.reject( - dependencyMessage(objectToPopulate.name, variableString, dependentService.name)); + dependencyMessage(variableString, dependentService.name)); }); - return this.populateValue(objectToPopulate.value, true) // populate - .then(populated => _.assign(objectToPopulate, { populated })) + return func() .finally(() => { dependentServices.forEach((dependentService) => { // restore this[dependentService.method] = dependentService.original; @@ -100,12 +99,16 @@ class Variables { prepopulateService() { const provider = this.serverless.getProvider('aws'); if (provider) { - const requiredConfig = [ + const requiredConfigs = [ _.assign({ name: 'region' }, provider.getRegionSourceValue()), _.assign({ name: 'stage' }, provider.getStageSourceValue()), ]; - const prepopulations = requiredConfig.map(this.prepopulateObject.bind(this)); - return this.assignProperties(provider, prepopulations); + return this.disableDepedentServices(() => { + const prepopulations = requiredConfigs.map(config => + this.populateValue(config.value, true) // populate + .then(populated => _.assign(config, { populated }))); + return this.assignProperties(provider, prepopulations); + }); } return BbPromise.resolve(); } diff --git a/lib/classes/Variables.test.js b/lib/classes/Variables.test.js index 8962814e3..5d523092a 100644 --- a/lib/classes/Variables.test.js +++ b/lib/classes/Variables.test.js @@ -169,7 +169,7 @@ describe('Variables', () => { it(`should reject ${config.name} variables in ${property.name} values`, () => { awsProvider.options[property.name] = config.value; return serverless.variables.populateService() - .should.be.rejectedWith('Variable Failure'); + .should.be.rejectedWith('Variable dependency failure'); }); it(`should reject recursively dependent ${config.name} service dependencies`, () => { serverless.variables.service.custom = { @@ -177,11 +177,36 @@ describe('Variables', () => { }; awsProvider.options.region = '${self:custom.settings.region}'; return serverless.variables.populateService() - .should.be.rejectedWith('Variable Failure'); + .should.be.rejectedWith('Variable dependency failure'); }); }); }); }); + describe('dependent service non-interference', () => { + const stateCombinations = [ + { region: 'foo', state: 'bar' }, + { region: 'foo', state: '${self:bar, "bar"}' }, + { region: '${self:foo, "foo"}', state: 'bar' }, + { region: '${self:foo, "foo"}', state: '${self:bar, "bar"}' }, + ]; + stateCombinations.forEach((combination) => { + it('must leave the dependent services in their original state', () => { + const dependentMethods = [ + { name: 'getValueFromCf', original: serverless.variables.getValueFromCf }, + { name: 'getValueFromS3', original: serverless.variables.getValueFromS3 }, + { name: 'getValueFromSsm', original: serverless.variables.getValueFromSsm }, + ]; + awsProvider.options.region = combination.region; + awsProvider.options.state = combination.state; + return serverless.variables.populateService().should.be.fulfilled + .then(() => { + dependentMethods.forEach((method) => { + expect(serverless.variables[method.name]).to.equal(method.original); + }); + }); + }); + }); + }); }); describe('#getProperties', () => {