From bc1a008b97f01a24a226ec6dd903dd094a450bf6 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Thu, 6 Aug 2020 15:16:23 +0100 Subject: [PATCH] fix(apigatewayv2): cyclic dependency between HttpApi and the lambda function (#9100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As part of Lambda proxy integration, a `AWS::Lambda::Permission` resource is created to provide the HTTP service permission to invoke the lambda function, creating the dependency, Permission → API. The API, on the other hand, needs to refer to the Function's ARN and hence creates the dependency, API → Function. However, when the lambda function and the HTTP API are placed in different stacks, this creates a cyclic dependency between these two stacks. A picture is worth a thousand words: ``` +--------------------------------------------------------------+ | Lambda stack | | | | +-------------------+ +-----------------+ | | | | via ARN | | | | | Lambda Permission +----------->| Lambda Function | | | | | | | | | +-------+-----------+ +-----------------+ | | | ^ | +------------|--------------------------------|----------------+ |via ARN |via ARN | | +------------|--------------------------------|-----------------+ | v | | | +-----------+ +----------+---------+ | | | | via ID | | | | | Http API |<--------------+ API Integration | | | | | | | | | +-----------+ +--------------------+ | | | | API Gateway stack | +---------------------------------------------------------------+ ``` The fix here is to move the Lambda Permission resource into the same stack as where the API integration is defined, thereby breaking the dependency cycle. Now the 'API Gateway stack' will depend one way on the 'Lambda stack'. fixes #9075 BREAKING CHANGE: The parameter for the method `bind()` on `IHttpRouteIntegration` has changed to accept one of type `HttpRouteIntegrationBindOptions`. The previous parameter `IHttpRoute` is now a property inside the new parameter under the key `route`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-apigatewayv2/lib/http/integration.ts | 21 ++- .../lib/http/integrations/http.ts | 6 +- .../lib/http/integrations/lambda.ts | 7 +- .../aws-apigatewayv2/lib/http/route.ts | 5 +- .../http/integ.custom-domain.expected.json | 148 +++++++++--------- .../integ.http-proxy.expected.json | 16 +- .../integ.lambda-proxy.expected.json | 16 +- .../test/http/integrations/lambda.test.ts | 17 +- 8 files changed, 139 insertions(+), 97 deletions(-) diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts index 7e9672ed5a786..236f533ee746e 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts @@ -112,6 +112,23 @@ export class HttpIntegration extends Resource implements IHttpIntegration { } } +/** + * Options to the HttpRouteIntegration during its bind operation. + */ +export interface HttpRouteIntegrationBindOptions { + /** + * The route to which this is being bound. + */ + readonly route: IHttpRoute; + + /** + * The current scope in which the bind is occurring. + * If the `HttpRouteIntegration` being bound creates additional constructs, + * this will be used as their parent scope. + */ + readonly scope: Construct; +} + /** * The interface that various route integration classes will inherit. */ @@ -119,7 +136,7 @@ export interface IHttpRouteIntegration { /** * Bind this integration to the route. */ - bind(route: IHttpRoute): HttpRouteIntegrationConfig; + bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig; } /** @@ -149,4 +166,4 @@ export interface HttpRouteIntegrationConfig { * @default - undefined */ readonly payloadFormatVersion: PayloadFormatVersion; -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/http.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/http.ts index b514b63f8056f..d5020d2d908ba 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/http.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/http.ts @@ -1,5 +1,5 @@ -import { HttpIntegrationType, HttpRouteIntegrationConfig, IHttpRouteIntegration, PayloadFormatVersion } from '../integration'; -import { HttpMethod, IHttpRoute } from '../route'; +import { HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, IHttpRouteIntegration, PayloadFormatVersion } from '../integration'; +import { HttpMethod } from '../route'; /** * Properties to initialize a new `HttpProxyIntegration`. @@ -24,7 +24,7 @@ export class HttpProxyIntegration implements IHttpRouteIntegration { constructor(private readonly props: HttpProxyIntegrationProps) { } - public bind(_: IHttpRoute): HttpRouteIntegrationConfig { + public bind(_: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig { return { method: this.props.method ?? HttpMethod.ANY, payloadFormatVersion: PayloadFormatVersion.VERSION_1_0, // 1.0 is required and is the only supported format diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/lambda.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/lambda.ts index 155b671fa79c2..30973a10b2ca0 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/lambda.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/integrations/lambda.ts @@ -1,8 +1,7 @@ import { ServicePrincipal } from '@aws-cdk/aws-iam'; import { IFunction } from '@aws-cdk/aws-lambda'; import { Stack } from '@aws-cdk/core'; -import { HttpIntegrationType, HttpRouteIntegrationConfig, IHttpRouteIntegration, PayloadFormatVersion } from '../integration'; -import { IHttpRoute } from '../route'; +import { HttpIntegrationType, HttpRouteIntegrationBindOptions, HttpRouteIntegrationConfig, IHttpRouteIntegration, PayloadFormatVersion } from '../integration'; /** * Lambda Proxy integration properties @@ -29,8 +28,10 @@ export class LambdaProxyIntegration implements IHttpRouteIntegration { constructor(private readonly props: LambdaProxyIntegrationProps) { } - public bind(route: IHttpRoute): HttpRouteIntegrationConfig { + public bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig { + const route = options.route; this.props.handler.addPermission(`${route.node.uniqueId}-Permission`, { + scope: options.scope, principal: new ServicePrincipal('apigateway.amazonaws.com'), sourceArn: Stack.of(route).formatArn({ service: 'execute-api', diff --git a/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts b/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts index 574b2c82275e8..2f65902a6aaee 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts @@ -120,7 +120,10 @@ export class HttpRoute extends Resource implements IHttpRoute { this.path = props.routeKey.path; let integration: HttpIntegration | undefined; - const config = props.integration.bind(this); + const config = props.integration.bind({ + route: this, + scope: this, + }); integration = new HttpIntegration(this, `${this.node.id}-Integration`, { httpApi: props.httpApi, integrationType: config.type, diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/integ.custom-domain.expected.json b/packages/@aws-cdk/aws-apigatewayv2/test/http/integ.custom-domain.expected.json index 4c9ada01d006c..2273bb18b02b4 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/integ.custom-domain.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/integ.custom-domain.expected.json @@ -4,27 +4,31 @@ "Type": "AWS::IAM::Role", "Properties": { "AssumeRolePolicyDocument": { - "Statement": [{ - "Action": "sts:AssumeRole", - "Effect": "Allow", - "Principal": { - "Service": "lambda.amazonaws.com" + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } } - }], + ], "Version": "2012-10-17" }, - "ManagedPolicyArns": [{ - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + "ManagedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] ] - ] - }] + } + ] } }, "echohandler8F648AB2": { @@ -46,44 +50,26 @@ "echohandlerServiceRole833A8F7A" ] }, - "echohandlerinteghttpproxyHttpProxyProdApiDefaultRoute20082F68PermissionBE86C6B3": { - "Type": "AWS::Lambda::Permission", + "DNFDC76583": { + "Type": "AWS::ApiGatewayV2::DomainName", "Properties": { - "Action": "lambda:InvokeFunction", - "FunctionName": { - "Fn::GetAtt": [ - "echohandler8F648AB2", - "Arn" - ] - }, - "Principal": "apigateway.amazonaws.com", - "SourceArn": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":execute-api:", - { - "Ref": "AWS::Region" - }, - ":", - { - "Ref": "AWS::AccountId" - }, - ":", - { - "Ref": "HttpProxyProdApi368B6161" - }, - "/*/*" - ] - ] - } + "DomainName": "apigv2.demo.com", + "DomainNameConfigurations": [ + { + "CertificateArn": "arn:aws:acm:us-east-1:111111111111:certificate", + "EndpointType": "REGIONAL" + } + ] } }, - "echohandlerinteghttpproxyDemoApiDefaultRoute050CFFE6PermissionD503E35E": { + "HttpProxyProdApi368B6161": { + "Type": "AWS::ApiGatewayV2::Api", + "Properties": { + "Name": "HttpProxyProdApi", + "ProtocolType": "HTTP" + } + }, + "HttpProxyProdApiDefaultRouteinteghttpproxyHttpProxyProdApiDefaultRoute20082F68PermissionB29E0EB7": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -112,7 +98,7 @@ }, ":", { - "Ref": "DemoApiE67238F8" + "Ref": "HttpProxyProdApi368B6161" }, "/*/*" ] @@ -120,23 +106,6 @@ } } }, - "DNFDC76583": { - "Type": "AWS::ApiGatewayV2::DomainName", - "Properties": { - "DomainName": "apigv2.demo.com", - "DomainNameConfigurations": [{ - "CertificateArn": "arn:aws:acm:us-east-1:111111111111:certificate", - "EndpointType": "REGIONAL" - }] - } - }, - "HttpProxyProdApi368B6161": { - "Type": "AWS::ApiGatewayV2::Api", - "Properties": { - "Name": "HttpProxyProdApi", - "ProtocolType": "HTTP" - } - }, "HttpProxyProdApiDefaultRouteDefaultRouteIntegration702F0DF7": { "Type": "AWS::ApiGatewayV2::Integration", "Properties": { @@ -236,6 +205,43 @@ "ProtocolType": "HTTP" } }, + "DemoApiDefaultRouteinteghttpproxyDemoApiDefaultRoute050CFFE6PermissionB1FE82E7": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "echohandler8F648AB2", + "Arn" + ] + }, + "Principal": "apigateway.amazonaws.com", + "SourceArn": { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":execute-api:", + { + "Ref": "AWS::Region" + }, + ":", + { + "Ref": "AWS::AccountId" + }, + ":", + { + "Ref": "DemoApiE67238F8" + }, + "/*/*" + ] + ] + } + } + }, "DemoApiDefaultRouteDefaultRouteIntegration714B9B03": { "Type": "AWS::ApiGatewayV2::Integration", "Properties": { @@ -326,4 +332,4 @@ "Value": "https://apigv2.demo.com/demo" } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.http-proxy.expected.json b/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.http-proxy.expected.json index 475ef14f158ef..d91a751447c01 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.http-proxy.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.http-proxy.expected.json @@ -50,7 +50,14 @@ "AlwaysSuccessServiceRole6DB8C2F6" ] }, - "AlwaysSuccessinteghttpproxyLambdaProxyApiDefaultRoute17D52FE1Permission3B39FF57": { + "LambdaProxyApi67594471": { + "Type": "AWS::ApiGatewayV2::Api", + "Properties": { + "Name": "LambdaProxyApi", + "ProtocolType": "HTTP" + } + }, + "LambdaProxyApiDefaultRouteinteghttpproxyLambdaProxyApiDefaultRoute17D52FE1Permission4875FF59": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -87,13 +94,6 @@ } } }, - "LambdaProxyApi67594471": { - "Type": "AWS::ApiGatewayV2::Api", - "Properties": { - "Name": "LambdaProxyApi", - "ProtocolType": "HTTP" - } - }, "LambdaProxyApiDefaultRouteDefaultRouteIntegration97D5250B": { "Type": "AWS::ApiGatewayV2::Integration", "Properties": { diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.lambda-proxy.expected.json b/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.lambda-proxy.expected.json index ff35db0f3c0d5..38a6929757755 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.lambda-proxy.expected.json +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/integ.lambda-proxy.expected.json @@ -50,7 +50,14 @@ "AlwaysSuccessServiceRole6DB8C2F6" ] }, - "AlwaysSuccessinteglambdaproxyLambdaProxyApiDefaultRoute59CA2390Permission90573BC0": { + "LambdaProxyApi67594471": { + "Type": "AWS::ApiGatewayV2::Api", + "Properties": { + "Name": "LambdaProxyApi", + "ProtocolType": "HTTP" + } + }, + "LambdaProxyApiDefaultRouteinteglambdaproxyLambdaProxyApiDefaultRoute59CA2390Permission07F93503": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -87,13 +94,6 @@ } } }, - "LambdaProxyApi67594471": { - "Type": "AWS::ApiGatewayV2::Api", - "Properties": { - "Name": "LambdaProxyApi", - "ProtocolType": "HTTP" - } - }, "LambdaProxyApiDefaultRouteDefaultRouteIntegration97D5250B": { "Type": "AWS::ApiGatewayV2::Integration", "Properties": { diff --git a/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/lambda.test.ts b/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/lambda.test.ts index 9984b8fdd9bff..8cdea2a3748c5 100644 --- a/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/lambda.test.ts +++ b/packages/@aws-cdk/aws-apigatewayv2/test/http/integrations/lambda.test.ts @@ -1,6 +1,6 @@ import '@aws-cdk/assert/jest'; import { Code, Function, Runtime } from '@aws-cdk/aws-lambda'; -import { Stack } from '@aws-cdk/core'; +import { App, Stack } from '@aws-cdk/core'; import { HttpApi, HttpRoute, HttpRouteKey, LambdaProxyIntegration, PayloadFormatVersion } from '../../../lib'; describe('LambdaProxyIntegration', () => { @@ -39,6 +39,21 @@ describe('LambdaProxyIntegration', () => { PayloadFormatVersion: '1.0', }); }); + + test('no dependency cycles', () => { + const app = new App(); + const lambdaStack = new Stack(app, 'lambdaStack'); + const fooFn = fooFunction(lambdaStack, 'Fn'); + + const apigwStack = new Stack(app, 'apigwStack'); + new HttpApi(apigwStack, 'httpApi', { + defaultIntegration: new LambdaProxyIntegration({ + handler: fooFn, + }), + }); + + expect(() => app.synth()).not.toThrow(); + }); }); function fooFunction(stack: Stack, id: string) {