diff --git a/lib/plugins/aws/package/compile/events/s3/index.js b/lib/plugins/aws/package/compile/events/s3/index.js index 7292535a5..2e195c1c7 100644 --- a/lib/plugins/aws/package/compile/events/s3/index.js +++ b/lib/plugins/aws/package/compile/events/s3/index.js @@ -288,6 +288,7 @@ class AwsCompileS3Events { const { compiledCloudFormationTemplate } = provider; const { Resources } = compiledCloudFormationTemplate; const iamRoleStatements = []; + let anyFuncUsesExistingS3Bucket = false; // used to keep track of the custom resources created for each bucket const bucketResources = {}; @@ -295,7 +296,6 @@ class AwsCompileS3Events { service.getAllFunctions().forEach(functionName => { let numEventsForFunc = 0; let currentBucketName = null; - let funcUsesExistingS3Bucket = false; const functionObj = service.getFunction(functionName); const FunctionName = functionObj.name; @@ -306,7 +306,7 @@ class AwsCompileS3Events { let rules = null; const bucket = event.s3.bucket; const notificationEvent = event.s3.event || 's3:ObjectCreated:*'; - funcUsesExistingS3Bucket = true; + anyFuncUsesExistingS3Bucket = true; if (!currentBucketName) { currentBucketName = bucket; @@ -385,29 +385,29 @@ class AwsCompileS3Events { } }); } - - if (funcUsesExistingS3Bucket) { - iamRoleStatements.push({ - Effect: 'Allow', - Resource: { - 'Fn::Join': [ - ':', - [ - 'arn', - { Ref: 'AWS::Partition' }, - 'lambda', - { Ref: 'AWS::Region' }, - { Ref: 'AWS::AccountId' }, - 'function', - FunctionName, - ], - ], - }, - Action: ['lambda:AddPermission', 'lambda:RemovePermission'], - }); - } }); + if (anyFuncUsesExistingS3Bucket) { + iamRoleStatements.push({ + Effect: 'Allow', + Resource: { + 'Fn::Join': [ + ':', + [ + 'arn', + { Ref: 'AWS::Partition' }, + 'lambda', + { Ref: 'AWS::Region' }, + { Ref: 'AWS::AccountId' }, + 'function', + '*', + ], + ], + }, + Action: ['lambda:AddPermission', 'lambda:RemovePermission'], + }); + } + // check if we need to add DependsOn clauses in case more than 1 // custom resources are created for one bucket (to avoid race conditions) if (Object.keys(bucketResources).length > 0) { diff --git a/lib/plugins/aws/package/compile/events/s3/index.test.js b/lib/plugins/aws/package/compile/events/s3/index.test.js index bc1127601..94079a643 100644 --- a/lib/plugins/aws/package/compile/events/s3/index.test.js +++ b/lib/plugins/aws/package/compile/events/s3/index.test.js @@ -7,6 +7,7 @@ const chai = require('chai'); const proxyquire = require('proxyquire').noCallThru(); const AwsProvider = require('../../../../provider/awsProvider'); const Serverless = require('../../../../../../Serverless'); +const runServerless = require('../../../../../../../test/utils/run-serverless'); const { expect } = chai; chai.use(require('sinon-chai')); @@ -535,7 +536,7 @@ describe('AwsCompileS3Events', () => { Ref: 'AWS::AccountId', }, 'function', - 'first', + '*', ], ], }, @@ -620,7 +621,7 @@ describe('AwsCompileS3Events', () => { Ref: 'AWS::AccountId', }, 'function', - 'second', + '*', ], ], }, @@ -726,7 +727,7 @@ describe('AwsCompileS3Events', () => { Ref: 'AWS::AccountId', }, 'function', - 'second', + '*', ], ], }, @@ -866,96 +867,6 @@ describe('AwsCompileS3Events', () => { Resources, } = awsCompileS3Events.serverless.service.provider.compiledCloudFormationTemplate; - expect(addCustomResourceToServiceStub).to.have.been.calledOnce; - expect(addCustomResourceToServiceStub.args[0][1]).to.equal('s3'); - expect(addCustomResourceToServiceStub.args[0][2]).to.deep.equal([ - { - Action: ['s3:PutBucketNotification', 's3:GetBucketNotification'], - Effect: 'Allow', - Resource: { - 'Fn::Join': [ - ':', - [ - 'arn', - { - Ref: 'AWS::Partition', - }, - 's3', - '', - '', - 'existing-s3-bucket', - ], - ], - }, - }, - { - Action: ['lambda:AddPermission', 'lambda:RemovePermission'], - Effect: 'Allow', - Resource: { - 'Fn::Join': [ - ':', - [ - 'arn', - { - Ref: 'AWS::Partition', - }, - 'lambda', - { - Ref: 'AWS::Region', - }, - { - Ref: 'AWS::AccountId', - }, - 'function', - 'first', - ], - ], - }, - }, - { - Action: ['s3:PutBucketNotification', 's3:GetBucketNotification'], - Effect: 'Allow', - Resource: { - 'Fn::Join': [ - ':', - [ - 'arn', - { - Ref: 'AWS::Partition', - }, - 's3', - '', - '', - 'existing-s3-bucket', - ], - ], - }, - }, - { - Action: ['lambda:AddPermission', 'lambda:RemovePermission'], - Effect: 'Allow', - Resource: { - 'Fn::Join': [ - ':', - [ - 'arn', - { - Ref: 'AWS::Partition', - }, - 'lambda', - { - Ref: 'AWS::Region', - }, - { - Ref: 'AWS::AccountId', - }, - 'function', - 'second', - ], - ], - }, - }, - ]); expect(Object.keys(Resources)).to.have.length(2); expect(Resources.FirstCustomS31).to.deep.equal({ Type: 'Custom::S3', @@ -1041,5 +952,38 @@ describe('AwsCompileS3Events', () => { return expect(() => awsCompileS3Events.existingS3Buckets()).to.throw('Only one S3 Bucket'); }); + + it('should create lambda permissions policy with wild card', async () => { + const { cfTemplate } = await runServerless({ + fixture: 's3', + cliArgs: ['package'], + }); + + const expectedResource = [ + 'arn', + { + Ref: 'AWS::Partition', + }, + 'lambda', + { + Ref: 'AWS::Region', + }, + { + Ref: 'AWS::AccountId', + }, + 'function', + '*', + ]; + + const lambdaPermissionsPolicies = cfTemplate.Resources.IamRoleCustomResourcesLambdaExecution.Properties.Policies[ + '0' + ].PolicyDocument.Statement.filter(x => x.Action[0].includes('AddPermission')); + + expect(lambdaPermissionsPolicies).to.have.length(1); + + const actualResource = lambdaPermissionsPolicies[0].Resource['Fn::Join'][1]; + + expect(actualResource).to.deep.equal(expectedResource); + }); }); });