Repository: HyperBrain/serverless-aws-alias (js)

Issue:
IAM role for alias is not removed
On removal of an alias, the IAM role for the alias will stay in the base stack. As a consequence the physical role resource does not get removed.

Expectation:
On removal of an alias all alias resources should be removed correctly.

Pull request description:
Remove superfluous iam roles
Fixes #50 

Retain only IAM roles of currently deployed aliases.
Make sure that only fresh data is used in unit tests. Required data must be cloned, as otherwise the cached version is modified by the tests and reloaded on subsequent ones.

Patch:
diff --git a/lib/stackops/lambdaRole.js b/lib/stackops/lambdaRole.js
index 06722e9..4eadbbf 100644
--- a/lib/stackops/lambdaRole.js
+++ b/lib/stackops/lambdaRole.js
@@ -51,13 +51,22 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
 		func.DependsOn[dependencyIndex] = roleName;
 	});
 
-	if (_.isEmpty(utils.findReferences(currentTemplate.Resources, 'IamRoleLambdaExecution')) && _.has(currentTemplate, 'Resources.IamRoleLambdaExecution')) {
+	if (_.has(currentTemplate, 'Resources.IamRoleLambdaExecution')) {
+		if (!_.isEmpty(utils.findReferences(currentTemplate.Resources, 'IamRoleLambdaExecution'))) {
+			stageStack.Resources.IamRoleLambdaExecution = currentTemplate.Resources.IamRoleLambdaExecution;
+		}
 		delete currentTemplate.Resources.IamRoleLambdaExecution;
 	}
 
-	// Import all defined roles from the current template (without overwriting)
-	const currentRoles = _.assign({}, _.pickBy(currentTemplate.Resources, (resource, name) => resource.Type === 'AWS::IAM::Role' && /^IamRoleLambdaExecution/.test(name)));
-	_.defaults(stageStack.Resources, currentRoles);
+	// Retain the roles of all currently deployed aliases
+	_.forEach(aliasStackTemplates, aliasTemplate => {
+		const alias = _.get(aliasTemplate, 'Outputs.ServerlessAliasName.Value');
+		const aliasRoleName = `IamRoleLambdaExecution${alias}`;
+		const aliasRole = _.get(currentTemplate, `Resources.${aliasRoleName}`);
+		if (alias && aliasRole) {
+			stageStack.Resources[aliasRoleName] = aliasRole;
+		}
+	});
 
 	return BbPromise.resolve([ currentTemplate, aliasStackTemplates, currentAliasStackTemplate ]);
 };


Test patch:
diff --git a/test/aliasRestructureStack.test.js b/test/aliasRestructureStack.test.js
index 6b733d9..72e9f3a 100644
--- a/test/aliasRestructureStack.test.js
+++ b/test/aliasRestructureStack.test.js
@@ -4,6 +4,7 @@
  */
 
 const getInstalledPath = require('get-installed-path');
+const _ = require('lodash');
 const BbPromise = require('bluebird');
 const chai = require('chai');
 const sinon = require('sinon');
@@ -36,14 +37,14 @@ describe('aliasRestructureStack', () => {
 			stage: 'myStage',
 			region: 'us-east-1',
 		};
-		serverless.setProvider('aws', new AwsProvider(serverless));
+		serverless.setProvider('aws', new AwsProvider(serverless, options));
 		serverless.cli = new serverless.classes.CLI(serverless);
 		serverless.service.service = 'testService';
 		serverless.service.provider.compiledCloudFormationAliasTemplate = {
 			Resources: {},
 			Outputs: {}
 		};
-		serverless.service.provider.compiledCloudFormationTemplate = require('./data/sls-stack-1.json');
+		serverless.service.provider.compiledCloudFormationTemplate = _.cloneDeep(require('./data/sls-stack-1.json'));
 		awsAlias = new AWSAlias(serverless, options);
 
 		// Disable logging
@@ -94,8 +95,8 @@ describe('aliasRestructureStack', () => {
 			const aliasHandleSNSEventsSpy = sandbox.spy(awsAlias, 'aliasHandleSNSEvents');
 			const aliasFinalizeSpy = sandbox.spy(awsAlias, 'aliasFinalize');
 
-			const currentTemplate = require('./data/sls-stack-2.json');
-			const aliasTemplate = require('./data/alias-stack-1.json');
+			const currentTemplate = _.cloneDeep(require('./data/sls-stack-2.json'));
+			const aliasTemplate = _.cloneDeep(require('./data/alias-stack-1.json'));
 			const currentAliasStackTemplate = {};
 
 			return expect(awsAlias.aliasRestructureStack(currentTemplate, [ aliasTemplate ], currentAliasStackTemplate))
diff --git a/test/configureAliasStack.test.js b/test/configureAliasStack.test.js
index f168799..69facba 100644
--- a/test/configureAliasStack.test.js
+++ b/test/configureAliasStack.test.js
@@ -4,6 +4,7 @@
  */
 
 const getInstalledPath = require('get-installed-path');
+const _ = require('lodash');
 const BbPromise = require('bluebird');
 const chai = require('chai');
 const sinon = require('sinon');
@@ -53,14 +54,16 @@ describe('configureAliasStack', () => {
 
 	describe('#configureAliasStack()', () => {
 		let readFileSyncStub;
+		let stack1;
 
 		beforeEach(() => {
 			readFileSyncStub = sandbox.stub(serverless.utils, 'readFileSync');
+			stack1 = _.cloneDeep(require('./data/sls-stack-1.json'));
 		});
 
 		it('should set alias reference and properties to CF templates', () => {
 			readFileSyncStub.returns(require('../lib/alias-cloudformation-template.json'));
-			serverless.service.provider.compiledCloudFormationTemplate = require('./data/sls-stack-1.json');
+			serverless.service.provider.compiledCloudFormationTemplate = stack1;
 			const cfTemplate = serverless.service.provider.compiledCloudFormationTemplate;
 
 			return expect(awsAlias.validate()).to.be.fulfilled
diff --git a/test/stackops/init.test.js b/test/stackops/init.test.js
index 451257b..a99283e 100644
--- a/test/stackops/init.test.js
+++ b/test/stackops/init.test.js
@@ -4,6 +4,7 @@
  */
 
 const getInstalledPath = require('get-installed-path');
+const _ = require('lodash');
 const BbPromise = require('bluebird');
 const chai = require('chai');
 const sinon = require('sinon');
@@ -53,8 +54,8 @@ describe('SNS Events', () => {
 
 	describe('#aliasInit()', () => {
 		it('should set alias flags', () => {
-			serverless.service.provider.compiledCloudFormationTemplate = require('../data/sls-stack-1.json');
-			const aliasStack = serverless.service.provider.compiledCloudFormationAliasTemplate = require('../data/alias-stack-1.json');
+			serverless.service.provider.compiledCloudFormationTemplate = _.cloneDeep(require('../data/sls-stack-1.json'));
+			const aliasStack = serverless.service.provider.compiledCloudFormationAliasTemplate = _.cloneDeep(require('../data/alias-stack-1.json'));
 			return expect(awsAlias.aliasInit({}, [], {})).to.be.fulfilled
 			.then(() => BbPromise.all([
 				expect(aliasStack).to.have.property('Outputs')
diff --git a/test/stackops/lambdaRole.test.js b/test/stackops/lambdaRole.test.js
index 32b64b9..f2c9128 100644
--- a/test/stackops/lambdaRole.test.js
+++ b/test/stackops/lambdaRole.test.js
@@ -4,6 +4,7 @@
  */
 
 const getInstalledPath = require('get-installed-path');
+const _ = require('lodash');
 const chai = require('chai');
 const sinon = require('sinon');
 const AWSAlias = require('../../index');
@@ -34,8 +35,8 @@ describe('lambdaRole', () => {
 			stage: 'myStage',
 			region: 'us-east-1',
 		};
-		serverless = new Serverless(options);
-		serverless.setProvider('aws', new AwsProvider(serverless));
+		serverless = new Serverless();
+		serverless.setProvider('aws', new AwsProvider(serverless, options));
 		serverless.cli = new serverless.classes.CLI(serverless);
 		serverless.service.service = 'testService';
 		serverless.service.provider.compiledCloudFormationAliasTemplate = {};
@@ -44,6 +45,8 @@ describe('lambdaRole', () => {
 		// Disable logging
 		logStub = sandbox.stub(serverless.cli, 'log');
 		logStub.returns();
+
+		return awsAlias.validate();
 	});
 
 	afterEach(() => {
@@ -51,10 +54,50 @@ describe('lambdaRole', () => {
 	});
 
 	describe('#aliasHandleLambdaRole()', () => {
+		let stack;
+
+		beforeEach(() => {
+			stack = _.cloneDeep(require('../data/sls-stack-1.json'));
+		});
+
 		it('should succeed with standard template', () => {
-			serverless.service.provider.compiledCloudFormationTemplate = require('../data/sls-stack-1.json');
+			serverless.service.provider.compiledCloudFormationTemplate = stack;
 			return expect(awsAlias.aliasHandleLambdaRole({}, [], {})).to.be.fulfilled;
 		});
 
+		it('should remove old global IAM role when there are no references', () => {
+			const currentTemplate = {
+				Resources: {
+					IamRoleLambdaExecution: {}
+				},
+				Outputs: {}
+			};
+			serverless.service.provider.compiledCloudFormationTemplate = stack;
+			return expect(awsAlias.aliasHandleLambdaRole(currentTemplate, [], {})).to.be.fulfilled
+			.then(() => expect(currentTemplate).to.not.have.a.property('IamRoleLambdaExecution'));
+		});
+
+		it('should retain existing alias roles', () => {
+			const aliasTemplates = [{
+				Resources: {},
+				Outputs: {
+					ServerlessAliasName: {
+						Description: 'The current alias',
+						Value: 'testAlias'
+					}
+				}
+			}];
+			const currentTemplate = {
+				Resources: {
+					IamRoleLambdaExecution: {},
+					IamRoleLambdaExecutiontestAlias: {}
+				},
+				Outputs: {}
+			};
+			const stackTemplate = serverless.service.provider.compiledCloudFormationTemplate = stack;
+			return expect(awsAlias.aliasHandleLambdaRole(currentTemplate, aliasTemplates, {})).to.be.fulfilled
+			.then(() => expect(stackTemplate).to.have.a.deep.property('Resources.IamRoleLambdaExecutiontestAlias'));
+		});
+
 	});
 });
diff --git a/test/stackops/snsEvents.test.js b/test/stackops/snsEvents.test.js
index 7e5c3bd..0168f33 100644
--- a/test/stackops/snsEvents.test.js
+++ b/test/stackops/snsEvents.test.js
@@ -4,6 +4,7 @@
  */
 
 const getInstalledPath = require('get-installed-path');
+const _ = require('lodash');
 const BbPromise = require('bluebird');
 const chai = require('chai');
 const sinon = require('sinon');
@@ -52,15 +53,25 @@ describe('SNS Events', () => {
 	});
 
 	describe('#aliasHandleSNSEvents()', () => {
+		let stack1;
+		let aliasStack1;
+		let snsStack1;
+
+		beforeEach(() => {
+			stack1 = _.cloneDeep(require('../data/sls-stack-1.json'));
+			aliasStack1 = _.cloneDeep(require('../data/alias-stack-1.json'));
+			snsStack1 = _.cloneDeep(require('../data/sns-stack.json'));
+		});
+
 		it('should succeed with standard template', () => {
-			serverless.service.provider.compiledCloudFormationTemplate = require('../data/sls-stack-1.json');
-			serverless.service.provider.compiledCloudFormationAliasTemplate = require('../data/alias-stack-1.json');
+			serverless.service.provider.compiledCloudFormationTemplate = stack1;
+			serverless.service.provider.compiledCloudFormationAliasTemplate = aliasStack1;
 			return expect(awsAlias.aliasHandleSNSEvents({}, [], {})).to.be.fulfilled;
 		});
 
 		it('should move resources to alias stack', () => {
-			const snsStack = serverless.service.provider.compiledCloudFormationTemplate = require('../data/sns-stack.json');
-			const aliasStack = serverless.service.provider.compiledCloudFormationAliasTemplate = require('../data/alias-stack-1.json');
+			const snsStack = serverless.service.provider.compiledCloudFormationTemplate = snsStack1;
+			const aliasStack = serverless.service.provider.compiledCloudFormationAliasTemplate = aliasStack1;
 			return expect(awsAlias.aliasHandleSNSEvents({}, [], {})).to.be.fulfilled
 			.then(() => BbPromise.all([
 				expect(snsStack).to.not.have.property('SNSTopicSlstestprojecttopic'),
@@ -71,8 +82,8 @@ describe('SNS Events', () => {
 		});
 
 		it('should replace function with alias reference', () => {
-			serverless.service.provider.compiledCloudFormationTemplate = require('../data/sns-stack.json');
-			const aliasStack = serverless.service.provider.compiledCloudFormationAliasTemplate = require('../data/alias-stack-1.json');
+			serverless.service.provider.compiledCloudFormationTemplate = snsStack1;
+			const aliasStack = serverless.service.provider.compiledCloudFormationAliasTemplate = aliasStack1;
 			return expect(awsAlias.aliasHandleSNSEvents({}, [], {})).to.be.fulfilled
 			.then(() => BbPromise.all([
 				expect(aliasStack).to.not.have.property('SNSTopicSlstestprojecttopic')