fix(apigateway): Bug fixes for cross-stack use of APIGateway and Lambda

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 #3705, #3000
aws · Sep 10, 2019 · 361c8e7 · 361c8e7
1 parent c4054ce
commit 361c8e7
Show file tree

Hide file tree

Showing 10 changed files with 369 additions and 37 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';
@@ -128,12 +128,23 @@ class LatestDeploymentResource extends CfnDeployment {
     if (this.hashComponents.length > 0) {
       const md5 = crypto.createHash('md5');
       this.hashComponents
-        .map(c => stack.resolve(c))
+        .map(c => {
+          try {
+            // TODO: Remove the code in the try block with next major version release of CDK.
+            // It's here to be backwards compatible, i.e., prevent LogicalIds to change.
+            return stack.resolve(c);
+          } catch (e) {
+            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
@@ -1,5 +1,6 @@
 import iam = require('@aws-cdk/aws-iam');
 import lambda = require('@aws-cdk/aws-lambda');
+import cdk = require('@aws-cdk/core');
 import { IntegrationOptions } from '../integration';
 import { Method } from '../method';
 import { AwsIntegration } from './aws';
@@ -60,14 +61,14 @@ export class LambdaIntegration extends AwsIntegration {
     this.handler.addPermission(`ApiPermission.${desc}`, {
       principal,
       sourceArn: method.methodArn,
-    });
+    }, method.node.scope as cdk.Construct);
 
     // add permission to invoke from the console
     if (this.enableTest) {
       this.handler.addPermission(`ApiPermission.Test.${desc}`, {
         principal,
         sourceArn: method.testMethodArn
-      });
+      }, method.node.scope as cdk.Construct);
     }
   }
 }
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": {
+    "booksapibooksApiPermissionrestapibooksexamplebooksapi4538F335GETbooks9CD1395A": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -91,7 +91,7 @@
         }
       }
     },
-    "BooksHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksCC375808": {
+    "booksapibooksApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooks8207720B": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -128,7 +128,7 @@
         }
       }
     },
-    "BooksHandlerApiPermissionrestapibooksexamplebooksapi4538F335POSTbooksFDED8A87": {
+    "booksapibooksApiPermissionrestapibooksexamplebooksapi4538F335POSTbooks4B64D4A2": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -169,7 +169,7 @@
         }
       }
     },
-    "BooksHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335POSTbooks4667899F": {
+    "booksapibooksApiPermissionTestrestapibooksexamplebooksapi4538F335POSTbooks99DD50F5": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -256,7 +256,7 @@
         "BookHandlerServiceRole894768AD"
       ]
     },
-    "BookHandlerApiPermissionrestapibooksexamplebooksapi4538F335GETbooksbookidA10D3CE2": {
+    "booksapibooksbookidApiPermissionrestapibooksexamplebooksapi4538F335GETbooksbookidEC2C8EF7": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -297,7 +297,7 @@
         }
       }
     },
-    "BookHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksbookidAB5191B6": {
+    "booksapibooksbookidApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksbookid91952F8D": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -334,7 +334,7 @@
         }
       }
     },
-    "BookHandlerApiPermissionrestapibooksexamplebooksapi4538F335DELETEbooksbookidB3A85313": {
+    "booksapibooksbookidApiPermissionrestapibooksexamplebooksapi4538F335DELETEbooksbookid2A4ABB5E": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -375,7 +375,7 @@
         }
       }
     },
-    "BookHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335DELETEbooksbookid9308C830": {
+    "booksapibooksbookidApiPermissionTestrestapibooksexamplebooksapi4538F335DELETEbooksbookid4D2C5A2E": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -462,7 +462,7 @@
         "HelloServiceRole1E55EA16"
       ]
     },
-    "HelloApiPermissionrestapibooksexamplebooksapi4538F335ANYE385693C": {
+    "booksapiApiPermissionrestapibooksexamplebooksapi4538F335ANY5C694FC0": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -503,7 +503,7 @@
         }
       }
     },
-    "HelloApiPermissionTestrestapibooksexamplebooksapi4538F335ANY46B0DA7B": {
+    "booksapiApiPermissionTestrestapibooksexamplebooksapi4538F335ANY308363F4": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",

diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json
@@ -430,7 +430,7 @@
         "MyHandlerServiceRoleFFA06653"
       ]
     },
-    "MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1toys00F704BC": {
+    "myapiv1toysApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1toys4996AB00": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -471,7 +471,7 @@
         }
       }
     },
-    "MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1toysDBCC8082": {
+    "myapiv1toysApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1toys235E617F": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -508,7 +508,7 @@
         }
       }
     },
-    "MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1books96EB3DB8": {
+    "myapiv1booksApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1books569CDE64": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -549,7 +549,7 @@
         }
       }
     },
-    "MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1books906B3BB6": {
+    "myapiv1booksApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1books83C4BA55": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -586,7 +586,7 @@
         }
       }
     },
-    "MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4POSTv1booksA48C273B": {
+    "myapiv1booksApiPermissiontestapigatewayrestapimyapi1AE401C4POSTv1books67C7577A": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",
@@ -627,7 +627,7 @@
         }
       }
     },
-    "MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4POSTv1booksA566985D": {
+    "myapiv1booksApiPermissionTesttestapigatewayrestapimyapi1AE401C4POSTv1booksE8FF0DE5": {
       "Type": "AWS::Lambda::Permission",
       "Properties": {
         "Action": "lambda:InvokeFunction",