Split IAM Policy from IAM Role & Improve DependsOn for Streams

lib/plugins/aws/lib/naming.js

@@ -97,8 +97,15 @@ module.exports = {
   getRoleLogicalId() {
     return 'IamRoleLambdaExecution';
   },
-
   // Policy
+  getPolicyLogicalId() {
+    return 'IamRoleLambdaExecutionPolicy';
+  },
+  getCustomPolicyName() {
+    const policyName = this.getPolicyName();
+    policyName['Fn::Join'][1].push('custom');
+    return policyName;
+  },
   getPolicyName() {
     return { // TODO should probably have name ordered and altered as above - see AWS docs
       'Fn::Join': [

lib/plugins/aws/package/compile/events/stream/index.js

@@ -127,7 +127,7 @@ class AwsCompileStreamEvents {
               .getStreamLogicalId(functionName, streamType, streamName);
 
             const funcRole = functionObj.role || this.serverless.service.provider.role;
-            let dependsOn = '"IamRoleLambdaExecution"';
+            let dependsOn = `"${this.provider.naming.getPolicyLogicalId()}"`;
             if (funcRole) {
               if ( // check whether the custom role is an ARN
                 typeof funcRole === 'string' &&
@@ -201,12 +201,10 @@ class AwsCompileStreamEvents {
 
         // update the PolicyDocument statements (if default policy is used)
         if (this.serverless.service.provider.compiledCloudFormationTemplate
-          .Resources.IamRoleLambdaExecution) {
+          .Resources[this.provider.naming.getRoleLogicalId()]) {
           const statement = this.serverless.service.provider.compiledCloudFormationTemplate
-            .Resources
-            .IamRoleLambdaExecution
+            .Resources[this.provider.naming.getPolicyLogicalId()]
             .Properties
-            .Policies[0]
             .PolicyDocument
             .Statement;
           if (dynamodbStreamStatement.Resource.length) {

lib/plugins/aws/package/compile/events/stream/index.test.js

@@ -24,6 +24,13 @@ describe('AwsCompileStreamEvents', () => {
             ],
           },
         },
+        IamRoleLambdaExecutionPolicy: {
+          Properties: {
+            PolicyDocument: {
+              Statement: [],
+            },
+          },
+        },
       },
     };
     serverless.setProvider('aws', new AwsProvider(serverless));
@@ -331,7 +338,7 @@ describe('AwsCompileStreamEvents', () => {
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingDynamodbFoo
           .DependsOn
-        ).to.equal('IamRoleLambdaExecution');
+        ).to.equal('IamRoleLambdaExecutionPolicy');
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingDynamodbFoo
           .Properties.EventSourceArn
@@ -366,7 +373,7 @@ describe('AwsCompileStreamEvents', () => {
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingDynamodbBar
           .DependsOn
-        ).to.equal('IamRoleLambdaExecution');
+        ).to.equal('IamRoleLambdaExecutionPolicy');
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingDynamodbBar
           .Properties.EventSourceArn
@@ -395,7 +402,7 @@ describe('AwsCompileStreamEvents', () => {
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingDynamodbBaz
           .DependsOn
-        ).to.equal('IamRoleLambdaExecution');
+        ).to.equal('IamRoleLambdaExecutionPolicy');
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingDynamodbBaz
           .Properties.EventSourceArn
@@ -464,8 +471,8 @@ describe('AwsCompileStreamEvents', () => {
           { 'Fn::GetAtt': ['SomeDdbTable', 'StreamArn'] }
         );
         expect(awsCompileStreamEvents.serverless.service
-          .provider.compiledCloudFormationTemplate.Resources.IamRoleLambdaExecution
-          .Properties.Policies[0].PolicyDocument.Statement[0]
+          .provider.compiledCloudFormationTemplate.Resources.IamRoleLambdaExecutionPolicy
+          .Properties.PolicyDocument.Statement[0]
         ).to.deep.equal(
           {
             Action: [
@@ -588,7 +595,7 @@ describe('AwsCompileStreamEvents', () => {
 
         expect(awsCompileStreamEvents.serverless.service.provider
           .compiledCloudFormationTemplate.Resources
-          .IamRoleLambdaExecution.Properties.Policies[0]
+          .IamRoleLambdaExecutionPolicy.Properties
           .PolicyDocument.Statement
         ).to.deep.equal(iamRoleStatements);
       });
@@ -629,7 +636,7 @@ describe('AwsCompileStreamEvents', () => {
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingKinesisFoo
           .DependsOn
-        ).to.equal('IamRoleLambdaExecution');
+        ).to.equal('IamRoleLambdaExecutionPolicy');
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingKinesisFoo
           .Properties.EventSourceArn
@@ -664,7 +671,7 @@ describe('AwsCompileStreamEvents', () => {
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingKinesisBar
           .DependsOn
-        ).to.equal('IamRoleLambdaExecution');
+        ).to.equal('IamRoleLambdaExecutionPolicy');
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingKinesisBar
           .Properties.EventSourceArn
@@ -693,7 +700,7 @@ describe('AwsCompileStreamEvents', () => {
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingKinesisBaz
           .DependsOn
-        ).to.equal('IamRoleLambdaExecution');
+        ).to.equal('IamRoleLambdaExecutionPolicy');
         expect(awsCompileStreamEvents.serverless.service
           .provider.compiledCloudFormationTemplate.Resources.FirstEventSourceMappingKinesisBaz
           .Properties.EventSourceArn
@@ -749,7 +756,7 @@ describe('AwsCompileStreamEvents', () => {
 
         expect(awsCompileStreamEvents.serverless.service.provider
           .compiledCloudFormationTemplate.Resources
-          .IamRoleLambdaExecution.Properties.Policies[0]
+          .IamRoleLambdaExecutionPolicy.Properties
           .PolicyDocument.Statement
         ).to.deep.equal(iamRoleStatements);
       });
@@ -764,11 +771,11 @@ describe('AwsCompileStreamEvents', () => {
 
       awsCompileStreamEvents.compileStreamEvents();
 
-      // should be 1 because we've mocked the IamRoleLambdaExecution above
+      // should be 2 because we've mocked IAM Role and Policy above
       expect(
         Object.keys(awsCompileStreamEvents.serverless.service.provider
           .compiledCloudFormationTemplate.Resources).length
-      ).to.equal(1);
+      ).to.equal(2);
     });
 
     it('should not add the IAM role statements when stream events are not given', () => {

lib/plugins/aws/package/compile/functions/index.js

@@ -49,11 +49,11 @@ class AwsCompileFunctions {
         if (role.startsWith('arn:aws')) {
           // role is a statically definied iam arn
           compiledFunction.Properties.Role = role;
-        } else if (role === 'IamRoleLambdaExecution') {
+        } else if (role === this.provider.naming.getRoleLogicalId()) {
           // role is the default role generated by the framework
           compiledFunction.Properties.Role = { 'Fn::GetAtt': [role, 'Arn'] };
           compiledFunction.DependsOn = [
-            'IamRoleLambdaExecution',
+            this.provider.naming.getRoleLogicalId(),
           ];
         } else {
           // role is a Logical Role Name
@@ -152,7 +152,7 @@ class AwsCompileFunctions {
         if (splittedArn[0] === 'arn' && (splittedArn[2] === 'sns' || splittedArn[2] === 'sqs')) {
           const dlqType = splittedArn[2];
           const iamRoleLambdaExecution = this.serverless.service.provider
-            .compiledCloudFormationTemplate.Resources.IamRoleLambdaExecution;
+            .compiledCloudFormationTemplate.Resources[this.provider.naming.getRoleLogicalId()];
           let stmt;
 
           newFunction.Properties.DeadLetterConfig = {
@@ -176,7 +176,7 @@ class AwsCompileFunctions {
             return BbPromise.reject(new this.serverless.classes.Error(errorMessage));
           }
 
-          // update the PolicyDocument statements (if default policy is used)
+          // update the inline PolicyDocument statements (if default policy is used)
           if (iamRoleLambdaExecution) {
             iamRoleLambdaExecution.Properties.Policies[0].PolicyDocument.Statement.push(stmt);
           }
@@ -212,7 +212,7 @@ class AwsCompileFunctions {
         const splittedArn = arn.split(':');
         if (splittedArn[0] === 'arn' && (splittedArn[2] === 'kms')) {
           const iamRoleLambdaExecution = this.serverless.service.provider
-            .compiledCloudFormationTemplate.Resources.IamRoleLambdaExecution;
+            .compiledCloudFormationTemplate.Resources[this.provider.naming.getRoleLogicalId()];
 
           newFunction.Properties.KmsKeyArn = arn;
 
@@ -323,7 +323,7 @@ class AwsCompileFunctions {
     } else if ('role' in this.serverless.service.provider) {
       this.compileRole(newFunction, this.serverless.service.provider.role);
     } else {
-      this.compileRole(newFunction, 'IamRoleLambdaExecution');
+      this.compileRole(newFunction, this.provider.naming.getRoleLogicalId());
     }
 
     if (!functionObject.vpc) functionObject.vpc = {};

lib/plugins/aws/package/lib/iam-policy-lambda-execution-template.json

@@ -0,0 +1,13 @@
+{
+  "Type": "AWS::IAM::Policy",
+  "Properties": {
+    "PolicyName": "[TO BE REPLACED]",
+    "PolicyDocument": {
+      "Version": "2012-10-17",
+      "Statement": []
+    },
+    "Roles": [{
+      "Ref": "[TO BE REPLACED]"
+    }]
+  }
+}

lib/plugins/aws/package/lib/iam-role-lambda-execution-template.json

@@ -42,4 +42,4 @@
       }
     ]
   }
-}
+}

lib/plugins/aws/package/lib/mergeIamTemplates.js

@@ -75,14 +75,31 @@ module.exports = {
     iamRoleLambdaExecutionTemplate.Properties.RoleName = this.provider.naming.getRoleName();
     iamRoleLambdaExecutionTemplate.Properties.Policies[0]
       .PolicyName = this.provider.naming.getPolicyName();
-
     _.merge(
       this.serverless.service.provider.compiledCloudFormationTemplate.Resources,
       {
         [this.provider.naming.getRoleLogicalId()]: iamRoleLambdaExecutionTemplate,
       }
     );
 
+    const iamPolicyTemplate = this.serverless.utils.readFileSync(
+      path.join(this.serverless.config.serverlessPath,
+        'plugins',
+        'aws',
+        'package',
+        'lib',
+        'iam-policy-lambda-execution-template.json')
+    );
+    iamPolicyTemplate.Properties.PolicyName = this.provider.naming.getCustomPolicyName();
+    iamPolicyTemplate.Properties.Roles[0].Ref = this.provider.naming.getRoleLogicalId();
+
+    _.merge(
+      this.serverless.service.provider.compiledCloudFormationTemplate.Resources,
+      {
+        [this.provider.naming.getPolicyLogicalId()]: iamPolicyTemplate,
+      }
+    );
+
     this.serverless.service.getAllFunctions().forEach((functionName) => {
       const functionObject = this.serverless.service.getFunction(functionName);
 
@@ -112,18 +129,11 @@ module.exports = {
     });
 
     if (this.serverless.service.provider.iamRoleStatements) {
-      // add custom iam role statements
-      this.serverless.service.provider.compiledCloudFormationTemplate
-        .Resources[this.provider.naming.getRoleLogicalId()]
-        .Properties
-        .Policies[0]
-        .PolicyDocument
-        .Statement = this.serverless.service.provider.compiledCloudFormationTemplate
-          .Resources[this.provider.naming.getRoleLogicalId()]
-          .Properties
-          .Policies[0]
-          .PolicyDocument
-          .Statement.concat(this.serverless.service.provider.iamRoleStatements);
+      const iamPolicyDocument = this.serverless.service.provider.compiledCloudFormationTemplate
+        .Resources[this.provider.naming.getPolicyLogicalId()].Properties.PolicyDocument;
+      // add custom iam role statements to custom Policy
+      iamPolicyDocument.Statement = iamPolicyDocument.Statement
+        .concat(this.serverless.service.provider.iamRoleStatements);
     }
 
     if (this.serverless.service.provider.iamManagedPolicies) {

lib/plugins/aws/package/lib/mergeIamTemplates.test.js

@@ -148,11 +148,10 @@ describe('#mergeIamTemplates()', () => {
     return awsPackage.mergeIamTemplates()
       .then(() => {
         expect(awsPackage.serverless.service.provider.compiledCloudFormationTemplate
-          .Resources[awsPackage.provider.naming.getRoleLogicalId()]
+          .Resources[awsPackage.provider.naming.getPolicyLogicalId()]
           .Properties
-          .Policies[0]
           .PolicyDocument
-          .Statement[2]
+          .Statement[0]
         ).to.deep.equal(awsPackage.serverless.service.provider.iamRoleStatements[0]);
       });
   });

lib/plugins/aws/package/lib/saveCompiledTemplate.js

@@ -13,8 +13,20 @@ module.exports = {
       compiledTemplateFileName
     );
 
-    this.serverless.utils.writeFileSync(compiledTemplateFilePath,
-      this.serverless.service.provider.compiledCloudFormationTemplate);
+    const compiledTemplate = this.serverless.service.provider.compiledCloudFormationTemplate;
+    if (compiledTemplate.Resources[this.provider.naming.getPolicyLogicalId()]) {
+      const customIAMPolicyStatement = compiledTemplate
+        .Resources[this.provider.naming.getPolicyLogicalId()]
+        .Properties
+        .PolicyDocument
+        .Statement;
+      // remove custom IAM Policy if no custom statements (empty array would be invalid for CF)
+      if (!customIAMPolicyStatement.length) {
+        delete compiledTemplate.Resources[this.provider.naming.getPolicyLogicalId()];
+      }
+    }
+
+    this.serverless.utils.writeFileSync(compiledTemplateFilePath, compiledTemplate);
 
     return BbPromise.resolve();
   },

lib/plugins/aws/package/lib/saveCompiledTemplate.test.js

@@ -12,6 +12,7 @@ describe('#saveCompiledTemplate()', () => {
   let awsPackage;
   let getCompiledTemplateFileNameStub;
   let writeFileSyncStub;
+  let filePath;
 
   beforeEach(() => {
     const options = {};
@@ -21,9 +22,16 @@ describe('#saveCompiledTemplate()', () => {
     serverless.config.servicePath = 'my-service';
     serverless.service = {
       provider: {
-        compiledCloudFormationTemplate: 'compiled content',
+        compiledCloudFormationTemplate: {
+          Resources: {},
+        },
       },
     };
+    filePath = path.join(
+      awsPackage.serverless.config.servicePath,
+      '.serverless',
+      'compiled.json'
+    );
     getCompiledTemplateFileNameStub = sinon
       .stub(awsPackage.provider.naming, 'getCompiledTemplateFileName')
       .returns('compiled.json');
@@ -36,16 +44,31 @@ describe('#saveCompiledTemplate()', () => {
     awsPackage.serverless.utils.writeFileSync.restore();
   });
 
-  it('should write the compiled template to disk', () => {
-    const filePath = path.join(
-      awsPackage.serverless.config.servicePath,
-      '.serverless',
-      'compiled.json'
-    );
+  it('should write the compiled template to disk', () =>
+    awsPackage.saveCompiledTemplate().then(() => {
+      expect(getCompiledTemplateFileNameStub.calledOnce).to.equal(true);
+      const expectedInput = {
+        Resources: {},
+      };
+      expect(writeFileSyncStub.calledWithExactly(filePath, expectedInput)).to.equal(true);
+    })
+  );
 
+  it('should remove the custom IAM policy, if empty', () => {
+    serverless.service.provider.compiledCloudFormationTemplate.Resources
+      .IamRoleLambdaExecutionPolicy = {
+        Properties: {
+          PolicyDocument: {
+            Statement: [],
+          },
+        },
+      };
     return awsPackage.saveCompiledTemplate().then(() => {
       expect(getCompiledTemplateFileNameStub.calledOnce).to.equal(true);
-      expect(writeFileSyncStub.calledWithExactly(filePath, 'compiled content')).to.equal(true);
+      const expectedInput = {
+        Resources: {},
+      };
+      expect(writeFileSyncStub.calledWithExactly(filePath, expectedInput)).to.equal(true);
     });
   });
 });