fix(apigateway): cross-stack lambda integration causes a cyclic refer…

…ence (aws#4010) 1. Token resolution of Deployment construct must not resolve the entire stack, specifically during the prepare phase. stack.resolve() works only after the CDK app has been fully prepared. During the 'prepare' phase, token resolution should instead resolve the token partially and within the local context. 2. Scope the lambda.CfnPermission construct closer to the consumer of the permission rather than being closer to the lambda function. For instance, when a lambda function is being consumed by an APIGateway RestApi Method as a cross-stack reference, placing the lambda.CfnPermission construct closer to the RestApi Method reduces the possibility of cyclic dependencies. fixes aws#3705, aws#3000
Vboivin · Sep 12, 2019 · 17fc967 · 17fc967
1 parent dbf0134
commit 17fc967
Show file tree

Hide file tree

Showing 13 changed files with 729 additions and 408 deletions.
diff --git a/packages/@aws-cdk/aws-apigateway/lib/deployment.ts b/packages/@aws-cdk/aws-apigateway/lib/deployment.ts
@@ -1,4 +1,4 @@
-import { Construct, Lazy, RemovalPolicy, Resource, Stack } from '@aws-cdk/core';
+import { Construct, DefaultTokenResolver, Lazy, RemovalPolicy, Resource, Stack, StringConcat, Tokenization } from '@aws-cdk/core';
 import crypto = require('crypto');
 import { CfnDeployment, CfnDeploymentProps } from './apigateway.generated';
 import { IRestApi } from './restapi';
@@ -121,19 +121,22 @@ class LatestDeploymentResource extends CfnDeployment {
    * add via `addToLogicalId`.
    */
   protected prepare() {
-    const stack = Stack.of(this);
-
     // if hash components were added to the deployment, we use them to calculate
     // a logical ID for the deployment resource.
     if (this.hashComponents.length > 0) {
       const md5 = crypto.createHash('md5');
       this.hashComponents
-        .map(c => stack.resolve(c))
+        .map(c => {
+          return Tokenization.resolve(c, {
+            scope: this,
+            resolver: new DefaultTokenResolver(new StringConcat()),
+            preparing: true,
+          });
+        })
         .forEach(c => md5.update(JSON.stringify(c)));
 
       this.overrideLogicalId(this.originalLogicalId + md5.digest("hex"));
     }
-
     super.prepare();
   }
 }
diff --git a/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts b/packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts
@@ -59,14 +59,16 @@ export class LambdaIntegration extends AwsIntegration {
 
     this.handler.addPermission(`ApiPermission.${desc}`, {
       principal,
+      scope: method,
       sourceArn: method.methodArn,
     });
 
     // add permission to invoke from the console
     if (this.enableTest) {
       this.handler.addPermission(`ApiPermission.Test.${desc}`, {
         principal,
-        sourceArn: method.testMethodArn
+        scope: method,
+        sourceArn: method.testMethodArn,
       });
     }
   }

diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.books.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.books.expected.json
@@ -50,7 +50,7 @@
         "BooksHandlerServiceRole5B6A8847"
       ]
     },
-    "BooksHandlerApiPermissionrestapibooksexamplebooksapi4538F335GETbooks727D645E": {
+    "booksapibooksGETApiPermissionrestapibooksexamplebooksapi4538F335GETbooks391776D8": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -91,7 +91,7 @@
         }
       }
     },
-    "BooksHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksCC375808": {
+    "booksapibooksGETApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooks01FB3D1B": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -128,7 +128,7 @@
         }
       }
     },
-    "BooksHandlerApiPermissionrestapibooksexamplebooksapi4538F335POSTbooksFDED8A87": {
+    "booksapibooksPOSTApiPermissionrestapibooksexamplebooksapi4538F335POSTbooksDFEC643F": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -169,7 +169,7 @@
         }
       }
     },
-    "BooksHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335POSTbooks4667899F": {
+    "booksapibooksPOSTApiPermissionTestrestapibooksexamplebooksapi4538F335POSTbooks1C6D24C8": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -256,7 +256,7 @@
         "BookHandlerServiceRole894768AD"
       ]
     },
-    "BookHandlerApiPermissionrestapibooksexamplebooksapi4538F335GETbooksbookidA10D3CE2": {
+    "booksapibooksbookidGETApiPermissionrestapibooksexamplebooksapi4538F335GETbooksbookidBB91DFBD": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -297,7 +297,7 @@
         }
       }
     },
-    "BookHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksbookidAB5191B6": {
+    "booksapibooksbookidGETApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksbookidA0230B08": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -334,7 +334,7 @@
         }
       }
     },
-    "BookHandlerApiPermissionrestapibooksexamplebooksapi4538F335DELETEbooksbookidB3A85313": {
+    "booksapibooksbookidDELETEApiPermissionrestapibooksexamplebooksapi4538F335DELETEbooksbookid76C1C947": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -375,7 +375,7 @@
         }
       }
     },
-    "BookHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335DELETEbooksbookid9308C830": {
+    "booksapibooksbookidDELETEApiPermissionTestrestapibooksexamplebooksapi4538F335DELETEbooksbookid09D6CB8A": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -462,7 +462,7 @@
         "HelloServiceRole1E55EA16"
       ]
     },
-    "HelloApiPermissionrestapibooksexamplebooksapi4538F335ANYE385693C": {
+    "booksapiANYApiPermissionrestapibooksexamplebooksapi4538F335ANY73B3CDDC": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -503,7 +503,7 @@
         }
       }
     },
-    "HelloApiPermissionTestrestapibooksexamplebooksapi4538F335ANY46B0DA7B": {
+    "booksapiANYApiPermissionTestrestapibooksexamplebooksapi4538F335ANYB0D7D8AC": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -546,7 +546,7 @@
         "Name": "books-api"
       }
     },
-    "booksapiDeployment308B08F1c828b08824c062376eba921738884f85": {
+    "booksapiDeployment308B08F1038c4647da4cb72c7574b891fb62b77f": {
       "Type": "AWS::ApiGateway::Deployment",
       "Properties": {
         "RestApiId": {
@@ -571,7 +571,7 @@
           "Ref": "booksapiE1885304"
         },
         "DeploymentId": {
-          "Ref": "booksapiDeployment308B08F1c828b08824c062376eba921738884f85"
+          "Ref": "booksapiDeployment308B08F1038c4647da4cb72c7574b891fb62b77f"
         },
         "StageName": "prod"
       }

diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json
@@ -6,7 +6,7 @@
         "Name": "my-api"
       }
     },
-    "myapiDeployment92F2CB4919460d935da8177bcfbc418506e514ff": {
+    "myapiDeployment92F2CB49a38a6824a90d7d4e6cc3a7f11357e163": {
       "Type": "AWS::ApiGateway::Deployment",
       "Properties": {
         "RestApiId": {
@@ -38,7 +38,7 @@
         "CacheClusterEnabled": true,
         "CacheClusterSize": "0.5",
         "DeploymentId": {
-          "Ref": "myapiDeployment92F2CB4919460d935da8177bcfbc418506e514ff"
+          "Ref": "myapiDeployment92F2CB49a38a6824a90d7d4e6cc3a7f11357e163"
         },
         "Description": "beta stage",
         "MethodSettings": [
@@ -430,7 +430,7 @@
         "MyHandlerServiceRoleFFA06653"
       ]
     },
-    "MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1toys00F704BC": {
+    "myapiv1toysGETApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1toysA829D1CC": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -471,7 +471,7 @@
         }
       }
     },
-    "MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1toysDBCC8082": {
+    "myapiv1toysGETApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1toys9E0BAE9F": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -508,7 +508,7 @@
         }
       }
     },
-    "MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1books96EB3DB8": {
+    "myapiv1booksGETApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1books484ACD3F": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -549,7 +549,7 @@
         }
       }
     },
-    "MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1books906B3BB6": {
+    "myapiv1booksGETApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1booksE255E31A": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -586,7 +586,7 @@
         }
       }
     },
-    "MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4POSTv1booksA48C273B": {
+    "myapiv1booksPOSTApiPermissiontestapigatewayrestapimyapi1AE401C4POSTv1books2B1BC62B": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -627,7 +627,7 @@
         }
       }
     },
-    "MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4POSTv1booksA566985D": {
+    "myapiv1booksPOSTApiPermissionTesttestapigatewayrestapimyapi1AE401C4POSTv1books816A6B37": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",