fix(AWS S3): Fix handling of lambda removal permissions (#8384)

This commit is contained in:
Oz Weiss 2020-10-12 14:39:17 +03:00 committed by GitHub
parent 9a33e62c83
commit c2d40ea63b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 116 deletions

View File

@ -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) {

View File

@ -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);
});
});
});