fix(event-targets): circular dependency when the lambda target is in …

…a different stack (#11217) The Lambda Permission resource causes a cyclic dependency when the rule is in a seprate stack from the lambda target for the rule. (a picture is worth a thousand words) ``` +-------------------+ +---------------+ |Lamda Stack | |Event Stack | | | | | | +----------+ | | +------+ | | | | | | | | | | | Function |<-----------------+ Rule | | | | | | | | | | | +----------+ | | +------+ | | ^ | | ^ | | | | | | | | +-----+------+ | | | | | | | | | | | | | Permission +--------------------+ | | | | | | | | +------------+ | | | | | | | +-------------------+ +---------------+ ``` The fix is to move the Permission resource into the event stack instead. fixes #10942 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Oct 30, 2020 · e21f249 · e21f249
1 parent 1097ec0
commit e21f249
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 173 deletions.
diff --git a/packages/@aws-cdk/aws-events-targets/lib/util.ts b/packages/@aws-cdk/aws-events-targets/lib/util.ts
@@ -1,7 +1,7 @@
 import * as events from '@aws-cdk/aws-events';
 import * as iam from '@aws-cdk/aws-iam';
 import * as lambda from '@aws-cdk/aws-lambda';
-import { Construct, IConstruct } from '@aws-cdk/core';
+import { Construct, ConstructNode, IConstruct } from '@aws-cdk/core';
 
 /**
  * Obtain the Role for the EventBridge event
@@ -27,9 +27,18 @@ export function singletonEventRole(scope: IConstruct, policyStatements: iam.Poli
  * Allows a Lambda function to be called from a rule
  */
 export function addLambdaPermission(rule: events.IRule, handler: lambda.IFunction): void {
+  let scope: Construct | undefined;
+  let node: ConstructNode = handler.permissionsNode;
+  if (rule instanceof Construct) {
+    // Place the Permission resource in the same stack as Rule rather than the Function
+    // This is to reduce circular dependency when the lambda handler and the rule are across stacks.
+    scope = rule;
+    node = rule.node;
+  }
   const permissionId = `AllowEventRule${rule.node.uniqueId}`;
-  if (!handler.permissionsNode.tryFindChild(permissionId)) {
+  if (!node.tryFindChild(permissionId)) {
     handler.addPermission(permissionId, {
+      scope,
       action: 'lambda:InvokeFunction',
       principal: new iam.ServicePrincipal('events.amazonaws.com'),
       sourceArn: rule.ruleArn,

diff --git a/packages/@aws-cdk/aws-events-targets/test/aws-api/integ.aws-api.expected.json b/packages/@aws-cdk/aws-events-targets/test/aws-api/integ.aws-api.expected.json
@@ -29,6 +29,25 @@
         ]
       }
     },
+    "ScheduleRuleAllowEventRuleawscdkawsapitargetintegScheduleRule51140722763E20C1": {
+      "Type": "AWS::Lambda::Permission",
+      "Properties": {
+        "Action": "lambda:InvokeFunction",
+        "FunctionName": {
+          "Fn::GetAtt": [
+            "AWSb4cf1abd4e4f4bc699441af7ccd9ec371511E620",
+            "Arn"
+          ]
+        },
+        "Principal": "events.amazonaws.com",
+        "SourceArn": {
+          "Fn::GetAtt": [
+            "ScheduleRuleDA5BD877",
+            "Arn"
+          ]
+        }
+      }
+    },
     "AWSb4cf1abd4e4f4bc699441af7ccd9ec37ServiceRole9FFE9C50": {
       "Type": "AWS::IAM::Role",
       "Properties": {
@@ -146,44 +165,6 @@
         "AWSb4cf1abd4e4f4bc699441af7ccd9ec37ServiceRole9FFE9C50"
       ]
     },
-    "AWSb4cf1abd4e4f4bc699441af7ccd9ec37AllowEventRuleawscdkawsapitargetintegScheduleRule511407226CC02048": {
-      "Type": "AWS::Lambda::Permission",
-      "Properties": {
-        "Action": "lambda:InvokeFunction",
-        "FunctionName": {
-          "Fn::GetAtt": [
-            "AWSb4cf1abd4e4f4bc699441af7ccd9ec371511E620",
-            "Arn"
-          ]
-        },
-        "Principal": "events.amazonaws.com",
-        "SourceArn": {
-          "Fn::GetAtt": [
-            "ScheduleRuleDA5BD877",
-            "Arn"
-          ]
-        }
-      }
-    },
-    "AWSb4cf1abd4e4f4bc699441af7ccd9ec37AllowEventRuleawscdkawsapitargetintegPatternRule3D38858113E3D24D": {
-      "Type": "AWS::Lambda::Permission",
-      "Properties": {
-        "Action": "lambda:InvokeFunction",
-        "FunctionName": {
-          "Fn::GetAtt": [
-            "AWSb4cf1abd4e4f4bc699441af7ccd9ec371511E620",
-            "Arn"
-          ]
-        },
-        "Principal": "events.amazonaws.com",
-        "SourceArn": {
-          "Fn::GetAtt": [
-            "PatternRule4AF6D328",
-            "Arn"
-          ]
-        }
-      }
-    },
     "PatternRule4AF6D328": {
       "Type": "AWS::Events::Rule",
       "Properties": {
@@ -216,6 +197,25 @@
           }
         ]
       }
+    },
+    "PatternRuleAllowEventRuleawscdkawsapitargetintegPatternRule3D388581AA4F776B": {
+      "Type": "AWS::Lambda::Permission",
+      "Properties": {
+        "Action": "lambda:InvokeFunction",
+        "FunctionName": {
+          "Fn::GetAtt": [
+            "AWSb4cf1abd4e4f4bc699441af7ccd9ec371511E620",
+            "Arn"
+          ]
+        },
+        "Principal": "events.amazonaws.com",
+        "SourceArn": {
+          "Fn::GetAtt": [
+            "PatternRule4AF6D328",
+            "Arn"
+          ]
+        }
+      }
     }
   },
   "Parameters": {
@@ -232,4 +232,4 @@
       "Description": "Artifact hash for asset \"4e52413f31cff0a335f5083fa6197a6cb61928644842d89026c42c2d2a98342e\""
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json
@@ -50,26 +50,25 @@
         "MyFuncServiceRole54065130"
       ]
     },
-    "MyFuncAllowEventRulelambdaeventsTimer0E6AB6D8E3B334A3": {
-      "Type": "AWS::Lambda::Permission",
+    "TimerBF6F831F": {
+      "Type": "AWS::Events::Rule",
       "Properties": {
-        "Action": "lambda:InvokeFunction",
-        "FunctionName": {
-          "Fn::GetAtt": [
-            "MyFunc8A243A2C",
-            "Arn"
-          ]
-        },
-        "Principal": "events.amazonaws.com",
-        "SourceArn": {
-          "Fn::GetAtt": [
-            "TimerBF6F831F",
-            "Arn"
-          ]
-        }
+        "ScheduleExpression": "rate(1 minute)",
+        "State": "ENABLED",
+        "Targets": [
+          {
+            "Arn": {
+              "Fn::GetAtt": [
+                "MyFunc8A243A2C",
+                "Arn"
+              ]
+            },
+            "Id": "Target0"
+          }
+        ]
       }
     },
-    "MyFuncAllowEventRulelambdaeventsTimer27F866A1E0669C645": {
+    "TimerAllowEventRulelambdaeventsTimer0E6AB6D890F582F4": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -82,16 +81,16 @@
         "Principal": "events.amazonaws.com",
         "SourceArn": {
           "Fn::GetAtt": [
-            "Timer2B6F162E9",
+            "TimerBF6F831F",
             "Arn"
           ]
         }
       }
     },
-    "TimerBF6F831F": {
+    "Timer2B6F162E9": {
       "Type": "AWS::Events::Rule",
       "Properties": {
-        "ScheduleExpression": "rate(1 minute)",
+        "ScheduleExpression": "rate(2 minutes)",
         "State": "ENABLED",
         "Targets": [
           {
@@ -106,22 +105,23 @@
         ]
       }
     },
-    "Timer2B6F162E9": {
-      "Type": "AWS::Events::Rule",
+    "Timer2AllowEventRulelambdaeventsTimer27F866A1E50659689": {
+      "Type": "AWS::Lambda::Permission",
       "Properties": {
-        "ScheduleExpression": "rate(2 minutes)",
-        "State": "ENABLED",
-        "Targets": [
-          {
-            "Arn": {
-              "Fn::GetAtt": [
-                "MyFunc8A243A2C",
-                "Arn"
-              ]
-            },
-            "Id": "Target0"
-          }
-        ]
+        "Action": "lambda:InvokeFunction",
+        "FunctionName": {
+          "Fn::GetAtt": [
+            "MyFunc8A243A2C",
+            "Arn"
+          ]
+        },
+        "Principal": "events.amazonaws.com",
+        "SourceArn": {
+          "Fn::GetAtt": [
+            "Timer2B6F162E9",
+            "Arn"
+          ]
+        }
       }
     }
   }

diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts
@@ -1,4 +1,4 @@
-import { countResources, expect, haveResource } from '@aws-cdk/assert';
+import '@aws-cdk/assert/jest';
 import * as events from '@aws-cdk/aws-events';
 import * as lambda from '@aws-cdk/aws-lambda';
 import * as cdk from '@aws-cdk/core';
@@ -23,7 +23,7 @@ test('use lambda as an event rule target', () => {
   // THEN
   const lambdaId = 'MyLambdaCCE802FB';
 
-  expect(stack).to(haveResource('AWS::Lambda::Permission', {
+  expect(stack).toHaveResource('AWS::Lambda::Permission', {
     Action: 'lambda:InvokeFunction',
     FunctionName: {
       'Fn::GetAtt': [
@@ -33,9 +33,9 @@ test('use lambda as an event rule target', () => {
     },
     Principal: 'events.amazonaws.com',
     SourceArn: { 'Fn::GetAtt': ['Rule4C995B7F', 'Arn'] },
-  }));
+  });
 
-  expect(stack).to(haveResource('AWS::Lambda::Permission', {
+  expect(stack).toHaveResource('AWS::Lambda::Permission', {
     Action: 'lambda:InvokeFunction',
     FunctionName: {
       'Fn::GetAtt': [
@@ -45,17 +45,17 @@ test('use lambda as an event rule target', () => {
     },
     Principal: 'events.amazonaws.com',
     SourceArn: { 'Fn::GetAtt': ['Rule270732244', 'Arn'] },
-  }));
+  });
 
-  expect(stack).to(countResources('AWS::Events::Rule', 2));
-  expect(stack).to(haveResource('AWS::Events::Rule', {
+  expect(stack).toCountResources('AWS::Events::Rule', 2);
+  expect(stack).toHaveResource('AWS::Events::Rule', {
     Targets: [
       {
         Arn: { 'Fn::GetAtt': [lambdaId, 'Arn'] },
         Id: 'Target0',
       },
     ],
-  }));
+  });
 });
 
 test('adding same lambda function as target mutiple times creates permission only once', () => {
@@ -75,7 +75,7 @@ test('adding same lambda function as target mutiple times creates permission onl
   }));
 
   // THEN
-  expect(stack).to(countResources('AWS::Lambda::Permission', 1));
+  expect(stack).toCountResources('AWS::Lambda::Permission', 1);
 });
 
 test('adding same singleton lambda function as target mutiple times creates permission only once', () => {
@@ -100,7 +100,30 @@ test('adding same singleton lambda function as target mutiple times creates perm
   }));
 
   // THEN
-  expect(stack).to(countResources('AWS::Lambda::Permission', 1));
+  expect(stack).toCountResources('AWS::Lambda::Permission', 1);
+});
+
+test('lambda handler and cloudwatch event across stacks', () => {
+  // GIVEN
+  const app = new cdk.App();
+  const lambdaStack = new cdk.Stack(app, 'LambdaStack');
+
+  const fn = new lambda.Function(lambdaStack, 'MyLambda', {
+    code: new lambda.InlineCode('foo'),
+    handler: 'bar',
+    runtime: lambda.Runtime.PYTHON_2_7,
+  });
+
+  const eventStack = new cdk.Stack(app, 'EventStack');
+  new events.Rule(eventStack, 'Rule', {
+    schedule: events.Schedule.rate(cdk.Duration.minutes(1)),
+    targets: [new targets.LambdaFunction(fn)],
+  });
+
+  expect(() => app.synth()).not.toThrow();
+
+  // the Permission resource should be in the event stack
+  expect(eventStack).toCountResources('AWS::Lambda::Permission', 1);
 });
 
 function newTestLambda(scope: constructs.Construct) {