Skip to content

Commit

Permalink
fix(apigatewayv2): cyclic dependency between HttpApi and the lambda f…
Browse files Browse the repository at this point in the history
…unction (#9100)

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*
  • Loading branch information
Niranjan Jayakar authored and Elad Ben-Israel committed Aug 10, 2020
1 parent a7ead62 commit bc1a008
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 97 deletions.
21 changes: 19 additions & 2 deletions packages/@aws-cdk/aws-apigatewayv2/lib/http/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,31 @@ 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.
*/
export interface IHttpRouteIntegration {
/**
* Bind this integration to the route.
*/
bind(route: IHttpRoute): HttpRouteIntegrationConfig;
bind(options: HttpRouteIntegrationBindOptions): HttpRouteIntegrationConfig;
}

/**
Expand Down Expand Up @@ -149,4 +166,4 @@ export interface HttpRouteIntegrationConfig {
* @default - undefined
*/
readonly payloadFormatVersion: PayloadFormatVersion;
}
}
Original file line number Diff line number Diff line change
@@ -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`.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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',
Expand Down
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-apigatewayv2/lib/http/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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",
Expand Down Expand Up @@ -112,31 +98,14 @@
},
":",
{
"Ref": "DemoApiE67238F8"
"Ref": "HttpProxyProdApi368B6161"
},
"/*/*"
]
]
}
}
},
"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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -326,4 +332,4 @@
"Value": "https://apigv2.demo.com/demo"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -87,13 +94,6 @@
}
}
},
"LambdaProxyApi67594471": {
"Type": "AWS::ApiGatewayV2::Api",
"Properties": {
"Name": "LambdaProxyApi",
"ProtocolType": "HTTP"
}
},
"LambdaProxyApiDefaultRouteDefaultRouteIntegration97D5250B": {
"Type": "AWS::ApiGatewayV2::Integration",
"Properties": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -87,13 +94,6 @@
}
}
},
"LambdaProxyApi67594471": {
"Type": "AWS::ApiGatewayV2::Api",
"Properties": {
"Name": "LambdaProxyApi",
"ProtocolType": "HTTP"
}
},
"LambdaProxyApiDefaultRouteDefaultRouteIntegration97D5250B": {
"Type": "AWS::ApiGatewayV2::Integration",
"Properties": {
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit bc1a008

Please sign in to comment.