fix(s3): access denied when adding an event notification to a s3 bucket (#4219)

* fix(s3): fix potential access denied when adding an event notification to a s3 bucket

Sometimes the custom resource finishes creating before the iam role of the lambda function is created, after which the custom resource directly executes the lambda function. This results in an access denied error.

fixes #3318

* fix(s3): update expected results with dependencies on iam role and default policy

* fix(s3): update expected results with dependencies on iam role and default policy for aws-lambda-event-sources package

Author: wwsno

packages/@aws-cdk/aws-lambda-event-sources/test/integ.s3.expected.json

@@ -73,9 +73,9 @@
       }
     },
     "B08E7C7AF": {
-      "DeletionPolicy": "Delete",
+      "Type": "AWS::S3::Bucket",
       "UpdateReplacePolicy": "Delete",
-      "Type": "AWS::S3::Bucket"
+      "DeletionPolicy": "Delete"
     },
     "BNotificationsEB8DA980": {
       "Type": "Custom::S3BucketNotifications",
@@ -187,7 +187,11 @@
         },
         "Runtime": "nodejs8.10",
         "Timeout": 300
-      }
+      },
+      "DependsOn": [
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
+      ]
     }
   }
-}
+}

packages/@aws-cdk/aws-s3-notifications/test/integ.notifications.expected.json

@@ -1,9 +1,9 @@
 {
   "Resources": {
     "Bucket83908E77": {
-      "DeletionPolicy": "Delete",
+      "Type": "AWS::S3::Bucket",
       "UpdateReplacePolicy": "Delete",
-      "Type": "AWS::S3::Bucket"
+      "DeletionPolicy": "Delete"
     },
     "BucketNotifications8F2E257D": {
       "Type": "Custom::S3BucketNotifications",
@@ -222,12 +222,16 @@
         },
         "Runtime": "nodejs8.10",
         "Timeout": 300
-      }
+      },
+      "DependsOn": [
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
+      ]
     },
     "Bucket25524B414": {
-      "DeletionPolicy": "Delete",
+      "Type": "AWS::S3::Bucket",
       "UpdateReplacePolicy": "Delete",
-      "Type": "AWS::S3::Bucket"
+      "DeletionPolicy": "Delete"
     },
     "Bucket2NotificationsD9BA2A77": {
       "Type": "Custom::S3BucketNotifications",
@@ -274,4 +278,4 @@
       ]
     }
   }
-}
+}

packages/@aws-cdk/aws-s3-notifications/test/lambda/integ.bucket-notifications.expected.json

@@ -246,7 +246,11 @@
         },
         "Runtime": "nodejs8.10",
         "Timeout": 300
-      }
+      },
+      "DependsOn": [
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
+      ]
     }
   }
 }

packages/@aws-cdk/aws-s3-notifications/test/sns/integ.sns-bucket-notifications.expected.json

@@ -205,7 +205,11 @@
         },
         "Runtime": "nodejs8.10",
         "Timeout": 300
-      }
+      },
+      "DependsOn": [
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
+      ]
     }
   }
 }

packages/@aws-cdk/aws-s3-notifications/test/sqs/integ.bucket-notifications.expected.json

@@ -192,7 +192,11 @@
         },
         "Runtime": "nodejs8.10",
         "Timeout": 300
-      }
+      },
+      "DependsOn": [
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
+        "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC"
+      ]
     },
     "Bucket25524B414": {
       "Type": "AWS::S3::Bucket",

packages/@aws-cdk/aws-s3/lib/notifications-resource/notifications-resource-handler.ts

@@ -84,6 +84,8 @@ export class NotificationsResourceHandler extends cdk.Construct {
       }
     });
 
+    resource.node.addDependency(role);
+
     this.functionArn = resource.getAtt('Arn').toString();
   }
 }

packages/@aws-cdk/aws-s3/test/test.notification.ts

@@ -1,4 +1,4 @@
-import { expect, haveResource } from '@aws-cdk/assert';
+import { expect, haveResource, haveResourceLike, ResourcePart } from '@aws-cdk/assert';
 import cdk = require('@aws-cdk/core');
 import { Test } from 'nodeunit';
 import s3 = require('../lib');
@@ -64,6 +64,34 @@ export = {
     test.done();
   },
 
+  'the notification lambda handler must depend on the role to prevent executing too early'(test: Test) {
+    const stack = new cdk.Stack();
+
+    const bucket = new s3.Bucket(stack, 'MyBucket');
+
+    bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
+      bind: () => ({
+        arn: 'ARN',
+        type: s3.BucketNotificationDestinationType.TOPIC
+      })
+    });
+
+    expect(stack).to(haveResourceLike('AWS::Lambda::Function', {
+      Type: "AWS::Lambda::Function",
+      Properties: {
+        Role: {
+          "Fn::GetAtt": [
+            "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC",
+            "Arn"
+          ]
+        },
+      }, DependsOn: [ "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36",
+      "BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC" ]
+     }, ResourcePart.CompleteDefinition ) );
+
+    test.done();
+  },
+
   'throws with multiple prefix rules in a filter'(test: Test) {
     const stack = new cdk.Stack();