Multiple event definitions for existing S3 bucket

serverless · Aug 2, 2019 · 7fb269f · 7fb269f
1 parent 7bd196e
commit 7fb269f
Show file tree

Hide file tree

Showing 8 changed files with 446 additions and 51 deletions.
diff --git a/lib/plugins/aws/customResources/resources/s3/handler.js b/lib/plugins/aws/customResources/resources/s3/handler.js
@@ -17,7 +17,7 @@ function handler(event, context) {
 
 function create(event, context) {
   const { Region, AccountId } = getEnvironment(context);
-  const { FunctionName, BucketName, BucketConfig } = event.ResourceProperties;
+  const { FunctionName, BucketName, BucketConfigs } = event.ResourceProperties;
 
   const lambdaArn = getLambdaArn(Region, AccountId, FunctionName);
 
@@ -31,14 +31,14 @@ function create(event, context) {
       region: Region,
       functionName: FunctionName,
       bucketName: BucketName,
-      bucketConfig: BucketConfig,
+      bucketConfigs: BucketConfigs,
     })
   );
 }
 
 function update(event, context) {
   const { Region, AccountId } = getEnvironment(context);
-  const { FunctionName, BucketName, BucketConfig } = event.ResourceProperties;
+  const { FunctionName, BucketName, BucketConfigs } = event.ResourceProperties;
 
   const lambdaArn = getLambdaArn(Region, AccountId, FunctionName);
 
@@ -47,7 +47,7 @@ function update(event, context) {
     region: Region,
     functionName: FunctionName,
     bucketName: BucketName,
-    bucketConfig: BucketConfig,
+    bucketConfigs: BucketConfigs,
   });
 }
 

diff --git a/lib/plugins/aws/customResources/resources/s3/lib/bucket.js b/lib/plugins/aws/customResources/resources/s3/lib/bucket.js
@@ -46,7 +46,7 @@ function getConfiguration(config) {
 }
 
 function updateConfiguration(config) {
-  const { lambdaArn, functionName, bucketName, bucketConfig, region } = config;
+  const { lambdaArn, functionName, bucketName, bucketConfigs, region } = config;
   const s3 = new AWS.S3({ region });
   const Bucket = bucketName;
 
@@ -56,16 +56,18 @@ function updateConfiguration(config) {
       conf => !conf.Id.startsWith(functionName)
     );
 
-    // add the event to the existing NotificationConfiguration
-    const Events = [bucketConfig.Event];
-    const LambdaFunctionArn = lambdaArn;
-    const Id = generateId(functionName, bucketConfig);
-    const Filter = createFilter(bucketConfig);
-    NotificationConfiguration.LambdaFunctionConfigurations.push({
-      Events,
-      LambdaFunctionArn,
-      Filter,
-      Id,
+    // add the events to the existing NotificationConfiguration
+    bucketConfigs.forEach(bucketConfig => {
+      const Events = [bucketConfig.Event];
+      const LambdaFunctionArn = lambdaArn;
+      const Id = generateId(functionName, bucketConfig);
+      const Filter = createFilter(bucketConfig);
+      NotificationConfiguration.LambdaFunctionConfigurations.push({
+        Events,
+        LambdaFunctionArn,
+        Filter,
+        Id,
+      });
     });
 
     const payload = {

diff --git a/lib/plugins/aws/lib/naming.js b/lib/plugins/aws/lib/naming.js
@@ -468,8 +468,11 @@ module.exports = {
       `${this.getNormalizedFunctionName(this.getCustomResourceS3HandlerFunctionName())}`
     );
   },
-  getCustomResourceS3ResourceLogicalId(functionName, idx) {
-    return `${this.getNormalizedFunctionName(functionName)}CustomS3${idx}`;
+  getCustomResourceS3ResourceLogicalId(functionName) {
+    // NOTE: we have to keep the 1 at the end to ensure backwards compatibility
+    // previously we've used an index to allow the creation of multiple custom S3 resources
+    // we're now using one resource to handle multiple S3 event definitions
+    return `${this.getNormalizedFunctionName(functionName)}CustomS31`;
   },
   // Cognito User Pool
   getCustomResourceCognitoUserPoolHandlerFunctionName() {

diff --git a/lib/plugins/aws/lib/naming.test.js b/lib/plugins/aws/lib/naming.test.js
@@ -755,8 +755,7 @@ describe('#naming()', () => {
   describe('#getCustomResourceS3ResourceLogicalId()', () => {
     it('should return the logical id of the S3 custom resouce', () => {
       const functionName = 'my-function';
-      const index = 1;
-      expect(sdk.naming.getCustomResourceS3ResourceLogicalId(functionName, index)).to.equal(
+      expect(sdk.naming.getCustomResourceS3ResourceLogicalId(functionName)).to.equal(
         'MyDashfunctionCustomS31'
       );
     });

diff --git a/lib/plugins/aws/package/compile/events/s3/index.js b/lib/plugins/aws/package/compile/events/s3/index.js
@@ -200,18 +200,23 @@ class AwsCompileS3Events {
     const { service } = this.serverless;
     const { provider } = service;
     const { compiledCloudFormationTemplate } = provider;
+    const { Resources } = compiledCloudFormationTemplate;
     const iamRoleStatements = [];
 
+    // used to keep track of the custom resources created for each bucket
+    const bucketResources = {};
+
     service.getAllFunctions().forEach(functionName => {
+      let numEventsForFunc = 0;
       let funcUsesExistingS3Bucket = false;
       const functionObj = service.getFunction(functionName);
       const FunctionName = functionObj.name;
 
       if (functionObj.events) {
-        functionObj.events.forEach((event, idx) => {
+        functionObj.events.forEach(event => {
           if (event.s3 && _.isObject(event.s3) && event.s3.existing) {
+            numEventsForFunc++;
             let rules = null;
-            idx = idx += 1;
             const bucket = event.s3.bucket;
             const notificationEvent = event.s3.event || 's3:ObjectCreated:*';
             funcUsesExistingS3Bucket = true;
@@ -227,38 +232,58 @@ class AwsCompileS3Events {
             const eventFunctionLogicalId = this.provider.naming.getLambdaLogicalId(functionName);
             const customResourceFunctionLogicalId = this.provider.naming.getCustomResourceS3HandlerFunctionLogicalId();
             const customS3ResourceLogicalId = this.provider.naming.getCustomResourceS3ResourceLogicalId(
-              functionName,
-              idx
+              functionName
             );
 
-            const customS3Resource = {
-              [customS3ResourceLogicalId]: {
-                Type: 'Custom::S3',
-                Version: 1.0,
-                DependsOn: [eventFunctionLogicalId, customResourceFunctionLogicalId],
-                Properties: {
-                  ServiceToken: {
-                    'Fn::GetAtt': [customResourceFunctionLogicalId, 'Arn'],
-                  },
-                  FunctionName,
-                  BucketName: bucket,
-                  BucketConfig: {
-                    Event: notificationEvent,
-                    Rules: rules,
+            // store how often the custom S3 resource is used
+            if (bucketResources[bucket]) {
+              bucketResources[bucket] = _.union(bucketResources[bucket], [
+                customS3ResourceLogicalId,
+              ]);
+            } else {
+              Object.assign(bucketResources, {
+                [bucket]: [customS3ResourceLogicalId],
+              });
+            }
+
+            let customS3Resource;
+            if (numEventsForFunc === 1) {
+              customS3Resource = {
+                [customS3ResourceLogicalId]: {
+                  Type: 'Custom::S3',
+                  Version: 1.0,
+                  DependsOn: [eventFunctionLogicalId, customResourceFunctionLogicalId],
+                  Properties: {
+                    ServiceToken: {
+                      'Fn::GetAtt': [customResourceFunctionLogicalId, 'Arn'],
+                    },
+                    FunctionName,
+                    BucketName: bucket,
+                    BucketConfigs: [
+                      {
+                        Event: notificationEvent,
+                        Rules: rules,
+                      },
+                    ],
                   },
                 },
-              },
-            };
+              };
 
-            _.merge(compiledCloudFormationTemplate.Resources, customS3Resource);
+              iamRoleStatements.push({
+                Effect: 'Allow',
+                Resource: {
+                  'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, `:s3:::${bucket}`]],
+                },
+                Action: ['s3:PutBucketNotification', 's3:GetBucketNotification'],
+              });
+            } else {
+              Resources[customS3ResourceLogicalId].Properties.BucketConfigs.push({
+                Event: notificationEvent,
+                Rules: rules,
+              });
+            }
 
-            iamRoleStatements.push({
-              Effect: 'Allow',
-              Resource: {
-                'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, `:s3:::${bucket}`]],
-              },
-              Action: ['s3:PutBucketNotification', 's3:GetBucketNotification'],
-            });
+            _.merge(Resources, customS3Resource);
           }
         });
       }
@@ -285,6 +310,22 @@ class AwsCompileS3Events {
       }
     });
 
+    // 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) {
+      Object.keys(bucketResources).forEach(bucket => {
+        const resources = bucketResources[bucket];
+        if (resources.length > 1) {
+          resources.forEach((currResourceLogicalId, idx) => {
+            if (idx > 0) {
+              const prevResourceLogicalId = resources[idx - 1];
+              Resources[currResourceLogicalId].DependsOn.push(prevResourceLogicalId);
+            }
+          });
+        }
+      });
+    }
+
     if (iamRoleStatements.length) {
       return addCustomResourceToService.call(this, 's3', iamRoleStatements);
     }