Skip to content

Commit

Permalink
fix(apigateway): cross-stack lambda integration fails token resolutio…
Browse files Browse the repository at this point in the history
…n and/or creates cyclic references

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
  • Loading branch information
Niranjan Jayakar committed Sep 12, 2019
1 parent dbf0134 commit a116edb
Show file tree
Hide file tree
Showing 13 changed files with 731 additions and 408 deletions.
13 changes: 8 additions & 5 deletions packages/@aws-cdk/aws-apigateway/lib/deployment.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
}
}
4 changes: 3 additions & 1 deletion packages/@aws-cdk/aws-apigateway/lib/integrations/lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"BooksHandlerServiceRole5B6A8847"
]
},
"BooksHandlerApiPermissionrestapibooksexamplebooksapi4538F335GETbooks727D645E": {
"booksapibooksGETApiPermissionrestapibooksexamplebooksapi4538F335GETbooks391776D8": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -91,7 +91,7 @@
}
}
},
"BooksHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksCC375808": {
"booksapibooksGETApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooks01FB3D1B": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -128,7 +128,7 @@
}
}
},
"BooksHandlerApiPermissionrestapibooksexamplebooksapi4538F335POSTbooksFDED8A87": {
"booksapibooksPOSTApiPermissionrestapibooksexamplebooksapi4538F335POSTbooksDFEC643F": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -169,7 +169,7 @@
}
}
},
"BooksHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335POSTbooks4667899F": {
"booksapibooksPOSTApiPermissionTestrestapibooksexamplebooksapi4538F335POSTbooks1C6D24C8": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -256,7 +256,7 @@
"BookHandlerServiceRole894768AD"
]
},
"BookHandlerApiPermissionrestapibooksexamplebooksapi4538F335GETbooksbookidA10D3CE2": {
"booksapibooksbookidGETApiPermissionrestapibooksexamplebooksapi4538F335GETbooksbookidBB91DFBD": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -297,7 +297,7 @@
}
}
},
"BookHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksbookidAB5191B6": {
"booksapibooksbookidGETApiPermissionTestrestapibooksexamplebooksapi4538F335GETbooksbookidA0230B08": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -334,7 +334,7 @@
}
}
},
"BookHandlerApiPermissionrestapibooksexamplebooksapi4538F335DELETEbooksbookidB3A85313": {
"booksapibooksbookidDELETEApiPermissionrestapibooksexamplebooksapi4538F335DELETEbooksbookid76C1C947": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -375,7 +375,7 @@
}
}
},
"BookHandlerApiPermissionTestrestapibooksexamplebooksapi4538F335DELETEbooksbookid9308C830": {
"booksapibooksbookidDELETEApiPermissionTestrestapibooksexamplebooksapi4538F335DELETEbooksbookid09D6CB8A": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -462,7 +462,7 @@
"HelloServiceRole1E55EA16"
]
},
"HelloApiPermissionrestapibooksexamplebooksapi4538F335ANYE385693C": {
"booksapiANYApiPermissionrestapibooksexamplebooksapi4538F335ANY73B3CDDC": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -503,7 +503,7 @@
}
}
},
"HelloApiPermissionTestrestapibooksexamplebooksapi4538F335ANY46B0DA7B": {
"booksapiANYApiPermissionTestrestapibooksexamplebooksapi4538F335ANYB0D7D8AC": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -546,7 +546,7 @@
"Name": "books-api"
}
},
"booksapiDeployment308B08F1c828b08824c062376eba921738884f85": {
"booksapiDeployment308B08F1038c4647da4cb72c7574b891fb62b77f": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
Expand All @@ -571,7 +571,7 @@
"Ref": "booksapiE1885304"
},
"DeploymentId": {
"Ref": "booksapiDeployment308B08F1c828b08824c062376eba921738884f85"
"Ref": "booksapiDeployment308B08F1038c4647da4cb72c7574b891fb62b77f"
},
"StageName": "prod"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"Name": "my-api"
}
},
"myapiDeployment92F2CB4919460d935da8177bcfbc418506e514ff": {
"myapiDeployment92F2CB49a38a6824a90d7d4e6cc3a7f11357e163": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
Expand Down Expand Up @@ -38,7 +38,7 @@
"CacheClusterEnabled": true,
"CacheClusterSize": "0.5",
"DeploymentId": {
"Ref": "myapiDeployment92F2CB4919460d935da8177bcfbc418506e514ff"
"Ref": "myapiDeployment92F2CB49a38a6824a90d7d4e6cc3a7f11357e163"
},
"Description": "beta stage",
"MethodSettings": [
Expand Down Expand Up @@ -430,7 +430,7 @@
"MyHandlerServiceRoleFFA06653"
]
},
"MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1toys00F704BC": {
"myapiv1toysGETApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1toysA829D1CC": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -471,7 +471,7 @@
}
}
},
"MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1toysDBCC8082": {
"myapiv1toysGETApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1toys9E0BAE9F": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -508,7 +508,7 @@
}
}
},
"MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1books96EB3DB8": {
"myapiv1booksGETApiPermissiontestapigatewayrestapimyapi1AE401C4GETv1books484ACD3F": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -549,7 +549,7 @@
}
}
},
"MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1books906B3BB6": {
"myapiv1booksGETApiPermissionTesttestapigatewayrestapimyapi1AE401C4GETv1booksE255E31A": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -586,7 +586,7 @@
}
}
},
"MyHandlerApiPermissiontestapigatewayrestapimyapi1AE401C4POSTv1booksA48C273B": {
"myapiv1booksPOSTApiPermissiontestapigatewayrestapimyapi1AE401C4POSTv1books2B1BC62B": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down Expand Up @@ -627,7 +627,7 @@
}
}
},
"MyHandlerApiPermissionTesttestapigatewayrestapimyapi1AE401C4POSTv1booksA566985D": {
"myapiv1booksPOSTApiPermissionTesttestapigatewayrestapimyapi1AE401C4POSTv1books816A6B37": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"Action": "lambda:InvokeFunction",
Expand Down
Loading

0 comments on commit a116edb

Please sign in to comment.